Skip to content

Commit 64f1995

Browse files
authored
Fix stack overflow in allPathsGoThroughCold past 6b11573 (llvm#106384)
Recursion here causes stack overflow on large inputs. Fixing by unrolling via a stack.
1 parent ce5620b commit 64f1995

File tree

1 file changed

+39
-41
lines changed

1 file changed

+39
-41
lines changed

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,54 +1762,52 @@ static void addNoReturnAttrs(const SCCNodeSet &SCCNodes,
17621762
}
17631763
}
17641764

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;
1765+
static bool allPathsGoThroughCold(Function &F) {
1766+
SmallDenseMap<BasicBlock *, bool, 16> ColdPaths;
1767+
ColdPaths[&F.front()] = false;
1768+
SmallVector<BasicBlock *> Jobs;
1769+
Jobs.push_back(&F.front());
1770+
1771+
while (!Jobs.empty()) {
1772+
BasicBlock *BB = Jobs.pop_back_val();
1773+
1774+
// If block contains a cold callsite this path through the CG is cold.
1775+
// Ignore whether the instructions actually are guaranteed to transfer
1776+
// execution. Divergent behavior is considered unlikely.
1777+
if (any_of(*BB, [](Instruction &I) {
1778+
if (auto *CB = dyn_cast<CallBase>(&I))
1779+
return CB->hasFnAttr(Attribute::Cold);
1780+
return false;
1781+
})) {
1782+
ColdPaths[BB] = true;
1783+
continue;
1784+
}
17841785

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;
1786+
auto Succs = successors(BB);
1787+
// We found a path that doesn't go through any cold callsite.
1788+
if (Succs.empty())
17971789
return false;
1790+
1791+
// We didn't find a cold callsite in this BB, so check that all successors
1792+
// contain a cold callsite (or that their successors do).
1793+
// Potential TODO: We could use static branch hints to assume certain
1794+
// successor paths are inherently cold, irrespective of if they contain a
1795+
// cold callsite.
1796+
for (BasicBlock *Succ : Succs) {
1797+
// Start with false, this is necessary to ensure we don't turn loops into
1798+
// cold.
1799+
auto [Iter, Inserted] = ColdPaths.try_emplace(Succ, false);
1800+
if (!Inserted) {
1801+
if (Iter->second)
1802+
continue;
1803+
return false;
1804+
}
1805+
Jobs.push_back(Succ);
17981806
}
1799-
if (!allBBPathsGoThroughCold(Succ, Visited))
1800-
return false;
1801-
Visited[Succ] = true;
18021807
}
1803-
18041808
return true;
18051809
}
18061810

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-
18131811
// Set the cold function attribute if possible.
18141812
static void addColdAttrs(const SCCNodeSet &SCCNodes,
18151813
SmallSet<Function *, 8> &Changed) {

0 commit comments

Comments
 (0)