[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 13:57:26 PDT 2025
https://github.com/alinas updated https://github.com/llvm/llvm-project/pull/150543
>From 5c8002cbe47c8cf25585e02a628acbafed8adef3 Mon Sep 17 00:00:00 2001
From: Alina Sbirlea <asbirlea at google.com>
Date: Thu, 24 Jul 2025 22:43:47 +0000
Subject: [PATCH 1/3] [MSSAUpdater] Replace recursion with worklist and cap it.
Replace the recursion used for finding and removing trivial MemoryPhis
with a worklist approach and add a cap for it for pathological cases.
This came up in a profile that also found #Issue150531, where a lot of
time was found on copying the MemoryPhi operands during the recusion.
---
llvm/include/llvm/Analysis/MemorySSAUpdater.h | 1 -
llvm/lib/Analysis/MemorySSAUpdater.cpp | 75 +++++++++++++++----
2 files changed, 60 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/Analysis/MemorySSAUpdater.h b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
index 96bf99922d848..8c03f241e8609 100644
--- a/llvm/include/llvm/Analysis/MemorySSAUpdater.h
+++ b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
@@ -257,7 +257,6 @@ class MemorySSAUpdater {
MemoryAccess *
getPreviousDefRecursive(BasicBlock *,
DenseMap<BasicBlock *, TrackingVH<MemoryAccess>> &);
- MemoryAccess *recursePhi(MemoryAccess *Phi);
MemoryAccess *tryRemoveTrivialPhi(MemoryPhi *Phi);
template <class RangeType>
MemoryAccess *tryRemoveTrivialPhi(MemoryPhi *Phi, RangeType &Operands);
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index ecfecb03c3757..56743e7e7caca 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -18,12 +18,17 @@
#include "llvm/Analysis/MemorySSA.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Dominators.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include <algorithm>
#define DEBUG_TYPE "memoryssa"
using namespace llvm;
+static cl::opt<uint32_t> TrivialPhiProcessingLimit(
+ "mssaupdater-processing-limit", cl::Hidden, cl::init(20),
+ cl::desc("The worklist limit for trivial phi removal (default = 20)"));
+
// This is the marker algorithm from "Simple and Efficient Construction of
// Static Single Assignment Form"
// The simple, non-marker algorithm places phi nodes at any join
@@ -181,18 +186,6 @@ MemoryAccess *MemorySSAUpdater::getPreviousDefFromEnd(
return getPreviousDefRecursive(BB, CachedPreviousDef);
}
-// Recurse over a set of phi uses to eliminate the trivial ones
-MemoryAccess *MemorySSAUpdater::recursePhi(MemoryAccess *Phi) {
- if (!Phi)
- return nullptr;
- TrackingVH<MemoryAccess> Res(Phi);
- SmallVector<TrackingVH<Value>, 8> Uses;
- std::copy(Phi->user_begin(), Phi->user_end(), std::back_inserter(Uses));
- for (auto &U : Uses)
- if (MemoryPhi *UsePhi = dyn_cast<MemoryPhi>(&*U))
- tryRemoveTrivialPhi(UsePhi);
- return Res;
-}
// Eliminate trivial phis
// Phis are trivial if they are defined either by themselves, or all the same
@@ -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.
+ if (NonOptPhis.count(RecPhi))
+ continue;
+
+ bool nextone = false;
+ MemoryAccess *RecSame = nullptr;
+ for (auto &Op : RecOperands) {
+ if (Op == RecPhi || Op == RecSame)
+ continue;
+ if (RecSame) {
+ nextone = true;
+ break;
+ }
+ RecSame = cast<MemoryAccess>(&*Op);
+ }
+
+ // Move on to the next Phi in the worklist.
+ if (nextone || RecSame == nullptr)
+ continue;
+
+ if (RecPhi) {
+ RecPhi->replaceAllUsesWith(RecSame);
+ removeMemoryAccess(RecPhi);
+ }
+
+ for (auto U : RecSame->users()) {
+ if (dyn_cast<MemoryPhi>(&*U))
+ Worklist.push_back(TrackingVH<Value>(U));
+ }
+ }
+
+ return Result;
}
void MemorySSAUpdater::insertUse(MemoryUse *MU, bool RenameUses) {
>From d25c8c05622e07129ed44fa4cc2222ea981c7310 Mon Sep 17 00:00:00 2001
From: Alina Sbirlea <asbirlea at google.com>
Date: Thu, 24 Jul 2025 22:50:47 +0000
Subject: [PATCH 2/3] Limit condition.
---
llvm/lib/Analysis/MemorySSAUpdater.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index 56743e7e7caca..2c28365e2a662 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -238,7 +238,7 @@ MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
Worklist.push_back(TrackingVH<Value>(U));
}
- while (!Worklist.empty() || Worklist.size() > TrivialPhiProcessingLimit) {
+ while (!Worklist.empty() && Worklist.size() < TrivialPhiProcessingLimit) {
MemoryPhi *RecPhi = dyn_cast<MemoryPhi>(&*(Worklist[Worklist.size() - 1]));
Worklist.pop_back();
>From 54f46c22c3b1bd74f0b76063942334d39bd0fec8 Mon Sep 17 00:00:00 2001
From: Alina Sbirlea <asbirlea at google.com>
Date: Mon, 28 Jul 2025 20:56:11 +0000
Subject: [PATCH 3/3] Remove ValueHandles.
---
llvm/lib/Analysis/MemorySSAUpdater.cpp | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index 2c28365e2a662..8810b891b26f4 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -228,21 +228,20 @@ MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
if (!Same)
return nullptr;
- TrackingVH<MemoryAccess> Result(Same);
-
// Worklist approach to recursively removing trivial Phis.
- SmallVector<TrackingVH<Value>, 5> Worklist;
+ SmallVector<Value*, 5> Worklist;
+ SmallSetVector<Value*, 5> Visited;
for (auto U : Same->users()) {
if (dyn_cast<MemoryPhi>(&*U))
- Worklist.push_back(TrackingVH<Value>(U));
+ Worklist.push_back(&*U);
}
while (!Worklist.empty() && Worklist.size() < TrivialPhiProcessingLimit) {
MemoryPhi *RecPhi = dyn_cast<MemoryPhi>(&*(Worklist[Worklist.size() - 1]));
Worklist.pop_back();
- if (!RecPhi)
+ if (!RecPhi || Visited.contains(RecPhi))
continue;
auto RecOperands = RecPhi->operands();
@@ -267,17 +266,22 @@ MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
continue;
if (RecPhi) {
+ if (RecPhi == Same) {
+ // Update the returned value
+ Same = RecSame;
+ }
+ Visited.insert(RecPhi);
RecPhi->replaceAllUsesWith(RecSame);
removeMemoryAccess(RecPhi);
}
for (auto U : RecSame->users()) {
if (dyn_cast<MemoryPhi>(&*U))
- Worklist.push_back(TrackingVH<Value>(U));
+ Worklist.push_back(&*U);
}
}
- return Result;
+ return Same;
}
void MemorySSAUpdater::insertUse(MemoryUse *MU, bool RenameUses) {
More information about the llvm-commits
mailing list