Skip to content

Commit 6b11573

Browse files
committed
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. Closes #105559
1 parent c1e401f commit 6b11573

File tree

3 files changed

+448
-193
lines changed

3 files changed

+448
-193
lines changed

compiler-rt/test/metadata/uar.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
33
// RUN: %clangxx %s -O1 -o %t -mcmodel=large -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
44

5-
// CHECK: metadata add version 2
5+
// CHECK-DAG: metadata add version 2
66

77
__attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
88
[[maybe_unused]] static const volatile void *sink;
@@ -14,51 +14,51 @@ __attribute__((noinline, not_tail_called)) void use(int x) {
1414
sink += x;
1515
}
1616

17-
// CHECK: empty: features=0 stack_args=0
17+
// CHECK-DAG: empty: features=0 stack_args=0
1818
void empty() {}
1919

20-
// CHECK: simple: features=0 stack_args=0
20+
// CHECK-DAG: simple: features=0 stack_args=0
2121
int simple(int *data, int index) { return data[index + 1]; }
2222

23-
// CHECK: builtins: features=0 stack_args=0
23+
// CHECK-DAG: builtins: features=0 stack_args=0
2424
int builtins() {
2525
int x = 0;
2626
__builtin_prefetch(&x);
2727
return x;
2828
}
2929

30-
// CHECK: ellipsis: features=0 stack_args=0
30+
// CHECK-DAG: ellipsis: features=0 stack_args=0
3131
void ellipsis(const char *fmt, ...) {
3232
int x;
3333
escape(&x);
3434
}
3535

36-
// CHECK: non_empty_function: features=2 stack_args=0
36+
// CHECK-DAG: non_empty_function: features=2 stack_args=0
3737
void non_empty_function() {
3838
int x;
3939
escape(&x);
4040
}
4141

42-
// CHECK: no_stack_args: features=2 stack_args=0
42+
// CHECK-DAG: no_stack_args: features=2 stack_args=0
4343
void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
4444
int x;
4545
escape(&x);
4646
}
4747

48-
// CHECK: stack_args: features=6 stack_args=16
48+
// CHECK-DAG: stack_args: features=6 stack_args=16
4949
void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
5050
int x;
5151
escape(&x);
5252
}
5353

54-
// CHECK: more_stack_args: features=6 stack_args=32
54+
// CHECK-DAG: more_stack_args: features=6 stack_args=32
5555
void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5,
5656
long a6, long a7, long a8) {
5757
int x;
5858
escape(&x);
5959
}
6060

61-
// CHECK: struct_stack_args: features=6 stack_args=144
61+
// CHECK-DAG: struct_stack_args: features=6 stack_args=144
6262
struct large {
6363
char x[131];
6464
};
@@ -69,28 +69,28 @@ void struct_stack_args(large a) {
6969

7070
__attribute__((noinline)) int tail_called(int x) { return x; }
7171

72-
// CHECK: with_tail_call: features=2
72+
// CHECK-DAG: with_tail_call: features=2
7373
int with_tail_call(int x) { [[clang::musttail]] return tail_called(x); }
7474

7575
__attribute__((noinline, noreturn)) int noreturn(int x) { __builtin_trap(); }
7676

77-
// CHECK: with_noreturn_tail_call: features=0
77+
// CHECK-DAG: with_noreturn_tail_call: features=0
7878
int with_noreturn_tail_call(int x) { return noreturn(x); }
7979

80-
// CHECK: local_array: features=0
80+
// CHECK-DAG: local_array: features=0
8181
void local_array(int x) {
8282
int data[10];
8383
use(data[x]);
8484
}
8585

86-
// CHECK: local_alloca: features=0
86+
// CHECK-DAG: local_alloca: features=0
8787
void local_alloca(int size, int i, int j) {
8888
volatile int *p = static_cast<int *>(__builtin_alloca(size));
8989
p[i] = 0;
9090
use(p[j]);
9191
}
9292

93-
// CHECK: escaping_alloca: features=2
93+
// CHECK-DAG: escaping_alloca: features=2
9494
void escaping_alloca(int size, int i) {
9595
volatile int *p = static_cast<int *>(__builtin_alloca(size));
9696
escape(&p[i]);

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
8282
STATISTIC(NumNoFree, "Number of functions marked as nofree");
8383
STATISTIC(NumWillReturn, "Number of functions marked as willreturn");
8484
STATISTIC(NumNoSync, "Number of functions marked as nosync");
85+
STATISTIC(NumCold, "Number of functions marked as cold");
8586

8687
STATISTIC(NumThinLinkNoRecurse,
8788
"Number of functions marked as norecurse during thinlink");
@@ -1745,6 +1746,7 @@ static bool canReturn(Function &F) {
17451746
return false;
17461747
}
17471748

1749+
17481750
// Set the noreturn function attribute if possible.
17491751
static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
17501752
SmallSet<Function *, 8> &Changed) {
@@ -1760,6 +1762,72 @@ static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
17601762
}
17611763
}
17621764

1765+
static bool
1766+
allBBPathsGoThroughCold(BasicBlock *BB,
1767+
SmallDenseMap<BasicBlock *, bool, 16> &Visited) {
1768+
// If BB contains a cold callsite this path through the CG is cold.
1769+
// Ignore whether the instructions actually are guranteed to transfer
1770+
// execution. Divergent behavior is considered unlikely.
1771+
if (any_of(*BB, [](Instruction &I) {
1772+
if (auto *CB = dyn_cast<CallBase>(&I))
1773+
return CB->hasFnAttr(Attribute::Cold);
1774+
return false;
1775+
})) {
1776+
Visited[BB] = true;
1777+
return true;
1778+
}
1779+
1780+
auto Succs = successors(BB);
1781+
// We found a path that doesn't go through any cold callsite.
1782+
if (Succs.empty())
1783+
return false;
1784+
1785+
// We didn't find a cold callsite in this BB, so check that all successors
1786+
// contain a cold callsite (or that their successors do).
1787+
// Potential TODO: We could use static branch hints to assume certain
1788+
// successor paths are inherently cold, irrespective of if they contain a cold
1789+
// callsite.
1790+
for (auto *Succ : Succs) {
1791+
// Start with false, this is necessary to ensure we don't turn loops into
1792+
// cold.
1793+
auto R = Visited.try_emplace(Succ, false);
1794+
if (!R.second) {
1795+
if (R.first->second)
1796+
continue;
1797+
return false;
1798+
}
1799+
if (!allBBPathsGoThroughCold(Succ, Visited))
1800+
return false;
1801+
Visited[Succ] = true;
1802+
}
1803+
1804+
return true;
1805+
}
1806+
1807+
static bool allPathsGoThroughCold(Function &F) {
1808+
SmallDenseMap<BasicBlock *, bool, 16> Visited;
1809+
Visited[&F.front()] = false;
1810+
return allBBPathsGoThroughCold(&F.front(), Visited);
1811+
}
1812+
1813+
// Set the cold function attribute if possible.
1814+
static void addColdAttrs(const SCCNodeSet &SCCNodes,
1815+
SmallSet<Function *, 8> &Changed) {
1816+
for (Function *F : SCCNodes) {
1817+
if (!F || !F->hasExactDefinition() || F->hasFnAttribute(Attribute::Naked) ||
1818+
F->hasFnAttribute(Attribute::Cold) || F->hasFnAttribute(Attribute::Hot))
1819+
continue;
1820+
1821+
// Potential TODO: We could add attribute `cold` on functions with `coldcc`.
1822+
if (allPathsGoThroughCold(*F)) {
1823+
F->addFnAttr(Attribute::Cold);
1824+
++NumCold;
1825+
Changed.insert(F);
1826+
continue;
1827+
}
1828+
}
1829+
}
1830+
17631831
static bool functionWillReturn(const Function &F) {
17641832
// We can infer and propagate function attributes only when we know that the
17651833
// definition we'll get at link time is *exactly* the definition we see now.
@@ -1853,6 +1921,7 @@ deriveAttrsInPostOrder(ArrayRef<Function *> Functions, AARGetterT &&AARGetter,
18531921
addArgumentAttrs(Nodes.SCCNodes, Changed);
18541922
inferConvergent(Nodes.SCCNodes, Changed);
18551923
addNoReturnAttrs(Nodes.SCCNodes, Changed);
1924+
addColdAttrs(Nodes.SCCNodes, Changed);
18561925
addWillReturn(Nodes.SCCNodes, Changed);
18571926
addNoUndefAttrs(Nodes.SCCNodes, Changed);
18581927

0 commit comments

Comments
 (0)