Recommit "[FunctionAttrs] deduce attr `cold` on functions if all CG paths call a `cold` function" #105559

goldsteinn · 2024-08-21T17:57:50Z

[CompilerRT] Refactor checks in uar.cpp test; NFC
Recommit "[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function"

Fixed up the uar test that was failing. It seems with the new cold
attribute the order of the functions is different. As far as I can
tell this is not a concern.

…aths call a `cold` function" Fixed up the uar test that was failing. It seems with the new `cold` attribute the order of the functions is different. As far as I can tell this is not a concern.

llvmbot · 2024-08-21T17:58:22Z

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

[CompilerRT] Refactor checks in uar.cpp test; NFC
Recommit "[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function"

Patch is 33.68 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105559.diff

3 Files Affected:

(modified) compiler-rt/test/metadata/uar.cpp (+31-15)
(modified) llvm/lib/Transforms/IPO/FunctionAttrs.cpp (+69)
(modified) llvm/test/Transforms/FunctionAttrs/cold.ll (+364-178)

diff --git a/compiler-rt/test/metadata/uar.cpp b/compiler-rt/test/metadata/uar.cpp
index cbafe462c3643c..4fbabfbf08d348 100644
--- a/compiler-rt/test/metadata/uar.cpp
+++ b/compiler-rt/test/metadata/uar.cpp
@@ -1,8 +1,38 @@
 // RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck %s
 // RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
-// RUN: %clangxx %s -O1 -o %t -mcmodel=large -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
+// RUN: %clangxx %s -O1 -o %t -mcmodel=large -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s --check-prefixes=CHECK-LARGE
 
 // CHECK: metadata add version 2
+// CHECK: with_noreturn_tail_call: features=0
+// CHECK: empty: features=0 stack_args=0
+// CHECK: simple: features=0 stack_args=0
+// CHECK: builtins: features=0 stack_args=0
+// CHECK: ellipsis: features=0 stack_args=0
+// CHECK: non_empty_function: features=2 stack_args=0
+// CHECK: no_stack_args: features=2 stack_args=0
+// CHECK: stack_args: features=6 stack_args=16
+// CHECK: more_stack_args: features=6 stack_args=32
+// CHECK: struct_stack_args: features=6 stack_args=144
+// CHECK: with_tail_call: features=2
+// CHECK: local_array: features=0
+// CHECK: local_alloca: features=0
+// CHECK: escaping_alloca: features=2
+
+// CHECK-LARGE: metadata add version 2
+// CHECK-LARGE: empty: features=0 stack_args=0
+// CHECK-LARGE: simple: features=0 stack_args=0
+// CHECK-LARGE: builtins: features=0 stack_args=0
+// CHECK-LARGE: ellipsis: features=0 stack_args=0
+// CHECK-LARGE: non_empty_function: features=2 stack_args=0
+// CHECK-LARGE: no_stack_args: features=2 stack_args=0
+// CHECK-LARGE: stack_args: features=6 stack_args=16
+// CHECK-LARGE: more_stack_args: features=6 stack_args=32
+// CHECK-LARGE: struct_stack_args: features=6 stack_args=144
+// CHECK-LARGE: with_tail_call: features=2
+// CHECK-LARGE: local_array: features=0
+// CHECK-LARGE: local_alloca: features=0
+// CHECK-LARGE: escaping_alloca: features=2
+// CHECK-LARGE: with_noreturn_tail_call: features=0
 
 __attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
   [[maybe_unused]] static const volatile void *sink;
@@ -14,51 +44,42 @@ __attribute__((noinline, not_tail_called)) void use(int x) {
   sink += x;
 }
 
-// CHECK: empty: features=0 stack_args=0
 void empty() {}
 
-// CHECK: simple: features=0 stack_args=0
 int simple(int *data, int index) { return data[index + 1]; }
 
-// CHECK: builtins: features=0 stack_args=0
 int builtins() {
   int x = 0;
   __builtin_prefetch(&x);
   return x;
 }
 
-// CHECK: ellipsis: features=0 stack_args=0
 void ellipsis(const char *fmt, ...) {
   int x;
   escape(&x);
 }
 
-// CHECK: non_empty_function: features=2 stack_args=0
 void non_empty_function() {
   int x;
   escape(&x);
 }
 
-// CHECK: no_stack_args: features=2 stack_args=0
 void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
   int x;
   escape(&x);
 }
 
-// CHECK: stack_args: features=6 stack_args=16
 void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
   int x;
   escape(&x);
 }
 
-// CHECK: more_stack_args: features=6 stack_args=32
 void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5,
                      long a6, long a7, long a8) {
   int x;
   escape(&x);
 }
 
-// CHECK: struct_stack_args: features=6 stack_args=144
 struct large {
   char x[131];
 };
@@ -69,28 +90,23 @@ void struct_stack_args(large a) {
 
 __attribute__((noinline)) int tail_called(int x) { return x; }
 
-// CHECK: with_tail_call: features=2
 int with_tail_call(int x) { [[clang::musttail]] return tail_called(x); }
 
 __attribute__((noinline, noreturn)) int noreturn(int x) { __builtin_trap(); }
 
-// CHECK: with_noreturn_tail_call: features=0
 int with_noreturn_tail_call(int x) { return noreturn(x); }
 
-// CHECK: local_array: features=0
 void local_array(int x) {
   int data[10];
   use(data[x]);
 }
 
-// CHECK: local_alloca: features=0
 void local_alloca(int size, int i, int j) {
   volatile int *p = static_cast<int *>(__builtin_alloca(size));
   p[i] = 0;
   use(p[j]);
 }
 
-// CHECK: escaping_alloca: features=2
 void escaping_alloca(int size, int i) {
   volatile int *p = static_cast<int *>(__builtin_alloca(size));
   escape(&p[i]);
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index d50218aaa3b6cc..603a1565e48c45 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -82,6 +82,7 @@ STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 STATISTIC(NumNoFree, "Number of functions marked as nofree");
 STATISTIC(NumWillReturn, "Number of functions marked as willreturn");
 STATISTIC(NumNoSync, "Number of functions marked as nosync");
+STATISTIC(NumCold, "Number of functions marked as cold");
 
 STATISTIC(NumThinLinkNoRecurse,
           "Number of functions marked as norecurse during thinlink");
@@ -1745,6 +1746,7 @@ static bool canReturn(Function &F) {
   return false;
 }
 
+
 // Set the noreturn function attribute if possible.
 static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
                              SmallSet<Function *, 8> &Changed) {
@@ -1760,6 +1762,72 @@ static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
   }
 }
 
+static bool
+allBBPathsGoThroughCold(BasicBlock *BB,
+                        SmallDenseMap<BasicBlock *, bool, 16> &Visited) {
+  // If BB contains a cold callsite this path through the CG is cold.
+  // Ignore whether the instructions actually are guranteed to transfer
+  // execution. Divergent behavior is considered unlikely.
+  if (any_of(*BB, [](Instruction &I) {
+        if (auto *CB = dyn_cast<CallBase>(&I))
+          return CB->hasFnAttr(Attribute::Cold);
+        return false;
+      })) {
+    Visited[BB] = true;
+    return true;
+  }
+
+  auto Succs = successors(BB);
+  // We found a path that doesn't go through any cold callsite.
+  if (Succs.empty())
+    return false;
+
+  // We didn't find a cold callsite in this BB, so check that all successors
+  // contain a cold callsite (or that their successors do).
+  // Potential TODO: We could use static branch hints to assume certain
+  // successor paths are inherently cold, irrespective of if they contain a cold
+  // callsite.
+  for (auto *Succ : Succs) {
+    // Start with false, this is necessary to ensure we don't turn loops into
+    // cold.
+    auto R = Visited.try_emplace(Succ, false);
+    if (!R.second) {
+      if (R.first->second)
+        continue;
+      return false;
+    }
+    if (!allBBPathsGoThroughCold(Succ, Visited))
+      return false;
+    Visited[Succ] = true;
+  }
+
+  return true;
+}
+
+static bool allPathsGoThroughCold(Function &F) {
+  SmallDenseMap<BasicBlock *, bool, 16> Visited;
+  Visited[&F.front()] = false;
+  return allBBPathsGoThroughCold(&F.front(), Visited);
+}
+
+// Set the cold function attribute if possible.
+static void addColdAttrs(const SCCNodeSet &SCCNodes,
+                         SmallSet<Function *, 8> &Changed) {
+  for (Function *F : SCCNodes) {
+    if (!F || !F->hasExactDefinition() || F->hasFnAttribute(Attribute::Naked) ||
+        F->hasFnAttribute(Attribute::Cold) || F->hasFnAttribute(Attribute::Hot))
+      continue;
+
+    // Potential TODO: We could add attribute `cold` on functions with `coldcc`.
+    if (allPathsGoThroughCold(*F)) {
+      F->addFnAttr(Attribute::Cold);
+      ++NumCold;
+      Changed.insert(F);
+      continue;
+    }
+  }
+}
+
 static bool functionWillReturn(const Function &F) {
   // We can infer and propagate function attributes only when we know that the
   // definition we'll get at link time is *exactly* the definition we see now.
@@ -1853,6 +1921,7 @@ deriveAttrsInPostOrder(ArrayRef<Function *> Functions, AARGetterT &&AARGetter,
   addArgumentAttrs(Nodes.SCCNodes, Changed);
   inferConvergent(Nodes.SCCNodes, Changed);
   addNoReturnAttrs(Nodes.SCCNodes, Changed);
+  addColdAttrs(Nodes.SCCNodes, Changed);
   addWillReturn(Nodes.SCCNodes, Changed);
   addNoUndefAttrs(Nodes.SCCNodes, Changed);
 
diff --git a/llvm/test/Transforms/FunctionAttrs/cold.ll b/llvm/test/Transforms/FunctionAttrs/cold.ll
index 1fa8ae06797943..a205fbda062121 100644
--- a/llvm/test/Transforms/FunctionAttrs/cold.ll
+++ b/llvm/test/Transforms/FunctionAttrs/cold.ll
@@ -54,14 +54,23 @@ while.body2:
 }
 
 define void @test_no_exit() {
-; COMMON: Function Attrs: noreturn
-; COMMON-LABEL: define void @test_no_exit
-; COMMON-SAME: () #[[ATTR2]] {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    br label [[WHILE_BODY:%.*]]
-; COMMON:       while.body:
-; COMMON-NEXT:    call void @cold0()
-; COMMON-NEXT:    br label [[WHILE_BODY]]
+; FNATTRS: Function Attrs: cold noreturn
+; FNATTRS-LABEL: define void @test_no_exit
+; FNATTRS-SAME: () #[[ATTR3:[0-9]+]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    br label [[WHILE_BODY:%.*]]
+; FNATTRS:       while.body:
+; FNATTRS-NEXT:    call void @cold0()
+; FNATTRS-NEXT:    br label [[WHILE_BODY]]
+;
+; ATTRIBUTOR: Function Attrs: noreturn
+; ATTRIBUTOR-LABEL: define void @test_no_exit
+; ATTRIBUTOR-SAME: () #[[ATTR2]] {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    br label [[WHILE_BODY:%.*]]
+; ATTRIBUTOR:       while.body:
+; ATTRIBUTOR-NEXT:    call void @cold0()
+; ATTRIBUTOR-NEXT:    br label [[WHILE_BODY]]
 ;
 entry:
   br label %while.body
@@ -72,17 +81,29 @@ while.body:
 }
 
 define void @test_no_exit2() {
-; COMMON: Function Attrs: noreturn
-; COMMON-LABEL: define void @test_no_exit2
-; COMMON-SAME: () #[[ATTR2]] {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    br label [[WHILE_BODY:%.*]]
-; COMMON:       while.body:
-; COMMON-NEXT:    call void @not_cold0()
-; COMMON-NEXT:    br label [[WHILE_BODY2:%.*]]
-; COMMON:       while.body2:
-; COMMON-NEXT:    call void @cold1()
-; COMMON-NEXT:    br label [[WHILE_BODY]]
+; FNATTRS: Function Attrs: cold noreturn
+; FNATTRS-LABEL: define void @test_no_exit2
+; FNATTRS-SAME: () #[[ATTR3]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    br label [[WHILE_BODY:%.*]]
+; FNATTRS:       while.body:
+; FNATTRS-NEXT:    call void @not_cold0()
+; FNATTRS-NEXT:    br label [[WHILE_BODY2:%.*]]
+; FNATTRS:       while.body2:
+; FNATTRS-NEXT:    call void @cold1()
+; FNATTRS-NEXT:    br label [[WHILE_BODY]]
+;
+; ATTRIBUTOR: Function Attrs: noreturn
+; ATTRIBUTOR-LABEL: define void @test_no_exit2
+; ATTRIBUTOR-SAME: () #[[ATTR2]] {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    br label [[WHILE_BODY:%.*]]
+; ATTRIBUTOR:       while.body:
+; ATTRIBUTOR-NEXT:    call void @not_cold0()
+; ATTRIBUTOR-NEXT:    br label [[WHILE_BODY2:%.*]]
+; ATTRIBUTOR:       while.body2:
+; ATTRIBUTOR-NEXT:    call void @cold1()
+; ATTRIBUTOR-NEXT:    br label [[WHILE_BODY]]
 ;
 entry:
   br label %while.body
@@ -97,18 +118,32 @@ while.body2:
 }
 
 define dso_local void @test_entry(i32 noundef %x) {
-; COMMON-LABEL: define dso_local void @test_entry
-; COMMON-SAME: (i32 noundef [[X:%.*]]) {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
-; COMMON:       if.then:
-; COMMON-NEXT:    tail call void @not_cold0()
-; COMMON-NEXT:    br label [[IF_END]]
-; COMMON:       if.end:
-; COMMON-NEXT:    tail call void @not_cold1()
-; COMMON-NEXT:    ret void
+; FNATTRS: Function Attrs: cold
+; FNATTRS-LABEL: define dso_local void @test_entry
+; FNATTRS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    tail call void @cold0()
+; FNATTRS-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; FNATTRS-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; FNATTRS:       if.then:
+; FNATTRS-NEXT:    tail call void @not_cold0()
+; FNATTRS-NEXT:    br label [[IF_END]]
+; FNATTRS:       if.end:
+; FNATTRS-NEXT:    tail call void @not_cold1()
+; FNATTRS-NEXT:    ret void
+;
+; ATTRIBUTOR-LABEL: define dso_local void @test_entry
+; ATTRIBUTOR-SAME: (i32 noundef [[X:%.*]]) {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    tail call void @cold0()
+; ATTRIBUTOR-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; ATTRIBUTOR-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; ATTRIBUTOR:       if.then:
+; ATTRIBUTOR-NEXT:    tail call void @not_cold0()
+; ATTRIBUTOR-NEXT:    br label [[IF_END]]
+; ATTRIBUTOR:       if.end:
+; ATTRIBUTOR-NEXT:    tail call void @not_cold1()
+; ATTRIBUTOR-NEXT:    ret void
 ;
 entry:
   tail call void @cold0()
@@ -125,12 +160,19 @@ if.end:
 }
 
 define dso_local void @test_hot_fail(i32 noundef %x) hot {
-; COMMON: Function Attrs: hot
-; COMMON-LABEL: define dso_local void @test_hot_fail
-; COMMON-SAME: (i32 noundef [[X:%.*]]) #[[ATTR3:[0-9]+]] {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    ret void
+; FNATTRS: Function Attrs: hot
+; FNATTRS-LABEL: define dso_local void @test_hot_fail
+; FNATTRS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR4:[0-9]+]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    tail call void @cold0()
+; FNATTRS-NEXT:    ret void
+;
+; ATTRIBUTOR: Function Attrs: hot
+; ATTRIBUTOR-LABEL: define dso_local void @test_hot_fail
+; ATTRIBUTOR-SAME: (i32 noundef [[X:%.*]]) #[[ATTR3:[0-9]+]] {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    tail call void @cold0()
+; ATTRIBUTOR-NEXT:    ret void
 ;
 entry:
   tail call void @cold0()
@@ -138,19 +180,34 @@ entry:
 }
 
 define dso_local void @test_br2(i32 noundef %x) {
-; COMMON-LABEL: define dso_local void @test_br2
-; COMMON-SAME: (i32 noundef [[X:%.*]]) {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
-; COMMON:       if.then:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    br label [[IF_END:%.*]]
-; COMMON:       if.else:
-; COMMON-NEXT:    tail call void @cold1()
-; COMMON-NEXT:    br label [[IF_END]]
-; COMMON:       if.end:
-; COMMON-NEXT:    ret void
+; FNATTRS: Function Attrs: cold
+; FNATTRS-LABEL: define dso_local void @test_br2
+; FNATTRS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR0]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; FNATTRS-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
+; FNATTRS:       if.then:
+; FNATTRS-NEXT:    tail call void @cold0()
+; FNATTRS-NEXT:    br label [[IF_END:%.*]]
+; FNATTRS:       if.else:
+; FNATTRS-NEXT:    tail call void @cold1()
+; FNATTRS-NEXT:    br label [[IF_END]]
+; FNATTRS:       if.end:
+; FNATTRS-NEXT:    ret void
+;
+; ATTRIBUTOR-LABEL: define dso_local void @test_br2
+; ATTRIBUTOR-SAME: (i32 noundef [[X:%.*]]) {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; ATTRIBUTOR-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
+; ATTRIBUTOR:       if.then:
+; ATTRIBUTOR-NEXT:    tail call void @cold0()
+; ATTRIBUTOR-NEXT:    br label [[IF_END:%.*]]
+; ATTRIBUTOR:       if.else:
+; ATTRIBUTOR-NEXT:    tail call void @cold1()
+; ATTRIBUTOR-NEXT:    br label [[IF_END]]
+; ATTRIBUTOR:       if.end:
+; ATTRIBUTOR-NEXT:    ret void
 ;
 entry:
   %tobool.not = icmp eq i32 %x, 0
@@ -169,21 +226,38 @@ if.end:
 }
 
 define dso_local void @test_exit(i32 noundef %x) {
-; COMMON-LABEL: define dso_local void @test_exit
-; COMMON-SAME: (i32 noundef [[X:%.*]]) {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    tail call void @not_cold0()
-; COMMON-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
-; COMMON:       if.then:
-; COMMON-NEXT:    tail call void @not_cold1()
-; COMMON-NEXT:    br label [[IF_END:%.*]]
-; COMMON:       if.else:
-; COMMON-NEXT:    tail call void @not_cold2()
-; COMMON-NEXT:    br label [[IF_END]]
-; COMMON:       if.end:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    ret void
+; FNATTRS: Function Attrs: cold
+; FNATTRS-LABEL: define dso_local void @test_exit
+; FNATTRS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR0]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    tail call void @not_cold0()
+; FNATTRS-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; FNATTRS-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
+; FNATTRS:       if.then:
+; FNATTRS-NEXT:    tail call void @not_cold1()
+; FNATTRS-NEXT:    br label [[IF_END:%.*]]
+; FNATTRS:       if.else:
+; FNATTRS-NEXT:    tail call void @not_cold2()
+; FNATTRS-NEXT:    br label [[IF_END]]
+; FNATTRS:       if.end:
+; FNATTRS-NEXT:    tail call void @cold0()
+; FNATTRS-NEXT:    ret void
+;
+; ATTRIBUTOR-LABEL: define dso_local void @test_exit
+; ATTRIBUTOR-SAME: (i32 noundef [[X:%.*]]) {
+; ATTRIBUTOR-NEXT:  entry:
+; ATTRIBUTOR-NEXT:    tail call void @not_cold0()
+; ATTRIBUTOR-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; ATTRIBUTOR-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN:%.*]]
+; ATTRIBUTOR:       if.then:
+; ATTRIBUTOR-NEXT:    tail call void @not_cold1()
+; ATTRIBUTOR-NEXT:    br label [[IF_END:%.*]]
+; ATTRIBUTOR:       if.else:
+; ATTRIBUTOR-NEXT:    tail call void @not_cold2()
+; ATTRIBUTOR-NEXT:    br label [[IF_END]]
+; ATTRIBUTOR:       if.end:
+; ATTRIBUTOR-NEXT:    tail call void @cold0()
+; ATTRIBUTOR-NEXT:    ret void
 ;
 entry:
   tail call void @not_cold0()
@@ -204,54 +278,104 @@ if.end:
 }
 
 define dso_local void @test_complex(i32 noundef %x) {
-; COMMON-LABEL: define dso_local void @test_complex
-; COMMON-SAME: (i32 noundef [[X:%.*]]) {
-; COMMON-NEXT:  entry:
-; COMMON-NEXT:    tail call void @not_cold0()
-; COMMON-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE11:%.*]], label [[IF_THEN:%.*]]
-; COMMON:       if.then:
-; COMMON-NEXT:    [[CALL:%.*]] = tail call i32 @get_val()
-; COMMON-NEXT:    [[TOBOOL1_NOT:%.*]] = icmp eq i32 [[CALL]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL1_NOT]], label [[IF_ELSE:%.*]], label [[IF_THEN2:%.*]]
-; COMMON:       if.then2:
-; COMMON-NEXT:    tail call void @cold1()
-; COMMON-NEXT:    br label [[IF_END12:%.*]]
-; COMMON:       if.else:
-; COMMON-NEXT:    [[CALL3:%.*]] = tail call i32 @get_val()
-; COMMON-NEXT:    [[TOBOOL4_NOT:%.*]] = icmp eq i32 [[CALL3]], 0
-; COMMON-NEXT:    br i1 [[TOBOOL4_NOT]], label [[IF_ELSE6:%.*]], label [[IF_THEN5:%.*]]
-; COMMON:       if.then5:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    br label [[IF_END12]]
-; COMMON:       if.else6:
-; COMMON-NEXT:    tail call void @not_cold0()
-; COMMON-NEXT:    [[CALL7:%.*]] = tail call i32 @get_val()
-; COMMON-NEXT:    switch i32 [[CALL7]], label [[SW_DEFAULT:%.*]] [
-; COMMON-NEXT:      i32 0, label [[SW_BB:%.*]]
-; COMMON-NEXT:      i32 1, label [[SW_BB8:%.*]]
-; COMMON-NEXT:      i32 2, label [[SW_BB9:%.*]]
-; COMMON-NEXT:    ]
-; COMMON:       sw.bb:
-; COMMON-NEXT:    tail call void @not_cold0()
-; COMMON-NEXT:    br label [[CALL_COLD:%.*]]
-; COMMON:       sw.bb8:
-; COMMON-NEXT:    tail call void @not_cold1()
-; COMMON-NEXT:    br label [[CALL_COLD]]
-; COMMON:       sw.bb9:
-; COMMON-NEXT:    tail call void @not_cold2()
-; COMMON-NEXT:    br label [[CALL_COLD]]
-; COMMON:       sw.default:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    br label [[IF_END12]]
-; COMMON:       call_cold:
-; COMMON-NEXT:    tail call void @cold_at_cb() #[[ATTR0:[0-9]+]]
-; COMMON-NEXT:    br label [[IF_END12]]
-; COMMON:       if.else11:
-; COMMON-NEXT:    tail call void @cold0()
-; COMMON-NEXT:    br label [[IF_END12]]
-; COMMON:       if.end12:
-; COMMON-NEXT:    ret void
+; FNATTRS: Function Attrs: cold
+; FNATTRS-LABEL: define dso_local void @test_complex
+; FNATTRS-SAME: (i32 noundef [[X:%.*]]) #[[ATTR0]] {
+; FNATTRS-NEXT:  entry:
+; FNATTRS-NEXT:    tail call void @not_cold0()
+; FNATTRS-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[X]], 0
+; FNATTRS-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_ELSE11:%.*]], label [[IF_THEN:%.*]]
+; FNATTRS:       if.then:
+; FNATTRS-NEXT:    [[CALL:%.*]] = tail call i32 @get_val()
+; FNATTRS-NEXT:    [[TOBOOL1_NOT:%.*]] = icmp eq i32 [[CALL]], 0
+; FNATTRS-NEXT:    br i1...
[truncated]

github-actions · 2024-08-21T18:01:35Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 69a0af756b921ad445eb9684f325d27a1c3a13b8 7abe294a44b3aa19bb1d10a14931a3464e815241 --extensions cpp -- compiler-rt/test/metadata/uar.cpp llvm/lib/Transforms/IPO/FunctionAttrs.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 603a1565e4..53568ae35d 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -1746,7 +1746,6 @@ static bool canReturn(Function &F) {
   return false;
 }
 
-
 // Set the noreturn function attribute if possible.
 static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
                              SmallSet<Function *, 8> &Changed) {

nikic · 2024-08-22T09:06:00Z

Fixed up the uar test that was failing. It seems with the new cold
attribute the order of the functions is different. As far as I can
tell this is not a concern.

If it's just an order difference, maybe it would be better to just use CHECK-DAG instead?

melver

LGTM for UAR test.
I leave it to you if you want to use CHECK-DAG or the current version. I'd err on the side of keeping it closer to the original version, as long as there won't be further changes that result in non-deterministic or constantly changing code gen.

goldsteinn · 2024-08-22T15:40:48Z

LGTM for UAR test. I leave it to you if you want to use CHECK-DAG or the current version. I'd err on the side of keeping it closer to the original version, as long as there won't be further changes that result in non-deterministic or constantly changing code gen.

I'm inclined to do CHECK-DAG. I wouldn't bet this is the last time the attributes will change and its a non-obvious failure.

…aths call a `cold` function" Fixed up the uar test that was failing. It seems with the new `cold` attribute the order of the functions is different. As far as I can tell this is not a concern. Closes llvm#105559

asmok-g · 2024-09-06T11:02:12Z

Heads-up: we're seeing probable miscompile in a google test after this commit. Working on a reproducer.

goldsteinn · 2024-09-09T18:02:14Z

Heads-up: we're seeing probable miscompile in a google test after this commit. Working on a reproducer.

Gotten a repro yet?

goldsteinn added 2 commits August 21, 2024 10:30

[CompilerRT] Refactor checks in uar.cpp test; NFC

b8c55e4

Recommit "[FunctionAttrs] deduce attr cold on functions if all CG p…

7abe294

…aths call a `cold` function" Fixed up the uar test that was failing. It seems with the new `cold` attribute the order of the functions is different. As far as I can tell this is not a concern.

goldsteinn mentioned this pull request Aug 21, 2024

[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function #101298

Closed

llvmbot added compiler-rt llvm:transforms labels Aug 21, 2024

goldsteinn requested review from dvyukov, melver, aeubanks, nikic and dtcxzyw August 21, 2024 17:58

goldsteinn changed the title ~~golsteinn/recommit func attr cold~~ Recommit "[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function" Aug 21, 2024

melver approved these changes Aug 22, 2024

View reviewed changes

goldsteinn closed this in 6b11573 Aug 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Recommit "[FunctionAttrs] deduce attr `cold` on functions if all CG paths call a `cold` function" #105559

Recommit "[FunctionAttrs] deduce attr `cold` on functions if all CG paths call a `cold` function" #105559

Uh oh!

goldsteinn commented Aug 21, 2024 •

edited

Loading

Uh oh!

llvmbot commented Aug 21, 2024

Uh oh!

github-actions bot commented Aug 21, 2024

Uh oh!

nikic commented Aug 22, 2024

Uh oh!

melver left a comment

Uh oh!

goldsteinn commented Aug 22, 2024

Uh oh!

asmok-g commented Sep 6, 2024

Uh oh!

goldsteinn commented Sep 9, 2024

Uh oh!

Uh oh!

Recommit "[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function" #105559

Recommit "[FunctionAttrs] deduce attr cold on functions if all CG paths call a cold function" #105559

Uh oh!

Conversation

goldsteinn commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 21, 2024

Uh oh!

github-actions bot commented Aug 21, 2024

Uh oh!

nikic commented Aug 22, 2024

Uh oh!

melver left a comment

Choose a reason for hiding this comment

Uh oh!

goldsteinn commented Aug 22, 2024

Uh oh!

asmok-g commented Sep 6, 2024

Uh oh!

goldsteinn commented Sep 9, 2024

Uh oh!

Uh oh!

Recommit "[FunctionAttrs] deduce attr `cold` on functions if all CG paths call a `cold` function" #105559

Recommit "[FunctionAttrs] deduce attr `cold` on functions if all CG paths call a `cold` function" #105559

goldsteinn commented Aug 21, 2024 •

edited

Loading