[llvm] Fix stack overflow in allPathsGoThroughCold past 6b11573b8c5e (PR #106384)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 29 16:42:23 PDT 2024
================
@@ -1762,54 +1763,53 @@ 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;
+static bool allPathsGoThroughCold(Function &F) {
+ SmallDenseMap<BasicBlock *, bool, 16> ColdPaths;
+ ColdPaths[&F.front()] = false;
+ std::stack<BasicBlock *> Jobs;
+ Jobs.push(&F.front());
+
+ while (!Jobs.empty()) {
+ BasicBlock *BB = Jobs.top();
+ Jobs.pop();
+
+ // If block contains a cold callsite this path through the CG is cold.
+ // Ignore whether the instructions actually are guaranteed 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;
+ })) {
+ ColdPaths[BB] = true;
+ continue;
+ }
- // 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;
+ 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 (llvm::BasicBlock *Succ : Succs) {
+ // Start with false, this is necessary to ensure we don't turn loops into
+ // cold.
+ auto [iter, inserted] = ColdPaths.try_emplace(Succ, false);
+ if (!inserted) {
+ if (iter->second)
----------------
goldsteinn wrote:
nit: style `Iter, Inserted`
https://github.com/llvm/llvm-project/pull/106384
More information about the llvm-commits
mailing list