[llvm] [MSSAUpdater] Replace recursion with worklist and cap it. (PR #150543)
Alina Sbirlea via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 28 11:56:31 PDT 2025
================
@@ -230,9 +223,61 @@ MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
removeMemoryAccess(Phi);
}
- // We should only end up recursing in case we replaced something, in which
- // case, we may have made other Phis trivial.
- return recursePhi(Same);
+ // Continue traversal in a DFS worklist approach, in case we might find
+ // other trivial Phis.
+ if (!Same)
+ return nullptr;
+
+ TrackingVH<MemoryAccess> Result(Same);
+
+ // Worklist approach to recursively removing trivial Phis.
+ SmallVector<TrackingVH<Value>, 5> Worklist;
+
+ for (auto U : Same->users()) {
+ if (dyn_cast<MemoryPhi>(&*U))
+ Worklist.push_back(TrackingVH<Value>(U));
+ }
+
+ while (!Worklist.empty() && Worklist.size() < TrivialPhiProcessingLimit) {
+ MemoryPhi *RecPhi = dyn_cast<MemoryPhi>(&*(Worklist[Worklist.size() - 1]));
+ Worklist.pop_back();
+
+ if (!RecPhi)
+ continue;
+ auto RecOperands = RecPhi->operands();
+
+ // Repeat above algorithm.
----------------
alinas wrote:
I think the issue here is that this API is used in two scenarios: 1) when looking at a normal MemoryPhi (called from `tryRemoveTrivialPhi` with the phi's operands), in which case this is just repeating the algorithm, and 2) when looking at an empty MemoryPhi with potential operands (during an update) and the goal is deciding whether to actually complete that Phi. This second case is also the only scenario where the return value is used.
Because the two cases have different types for how the list of operands is represented, this is templated to use a range, and this is also why this was originally a recursive function, so the range type can change (it only really changes at most once though).
I don't know how to resolve this without some duplication while avoiding copying the operands. Here's an alternative, but I'm open to suggestions.
```
// common case, public API
tryRemoveTrivialPhi(Phi) {
// calls tryRemoveTrivialPhiHelper and discards its return value, void
tryRemoveTrivialPhiHelper
}
// uncommon case, public API, remove template
tryRemoveIncompelteTrivialPhi(Phi, SmallVector<>& PhiOps) {
// duplicates the algorithm for the incoming range, before calling the helper
// returns the value from the helper
tryRemoveTrivialPhiHelper
}
// private API, returns a Phi
tryRemoveTrivialPhiHelper(Phi) {
// worklist algorithm using the Phi's operands, no repetition here
}
```
WDYT?
https://github.com/llvm/llvm-project/pull/150543
More information about the llvm-commits
mailing list