Skip to content

Commit 5c8002c

Browse files
committed
[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.
1 parent fd8f69d commit 5c8002c

File tree

2 files changed

+60
-16
lines changed

2 files changed

+60
-16
lines changed

llvm/include/llvm/Analysis/MemorySSAUpdater.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ class MemorySSAUpdater {
257257
MemoryAccess *
258258
getPreviousDefRecursive(BasicBlock *,
259259
DenseMap<BasicBlock *, TrackingVH<MemoryAccess>> &);
260-
MemoryAccess *recursePhi(MemoryAccess *Phi);
261260
MemoryAccess *tryRemoveTrivialPhi(MemoryPhi *Phi);
262261
template <class RangeType>
263262
MemoryAccess *tryRemoveTrivialPhi(MemoryPhi *Phi, RangeType &Operands);

llvm/lib/Analysis/MemorySSAUpdater.cpp

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,17 @@
1818
#include "llvm/Analysis/MemorySSA.h"
1919
#include "llvm/IR/BasicBlock.h"
2020
#include "llvm/IR/Dominators.h"
21+
#include "llvm/Support/CommandLine.h"
2122
#include "llvm/Support/Debug.h"
2223
#include <algorithm>
2324

2425
#define DEBUG_TYPE "memoryssa"
2526
using namespace llvm;
2627

28+
static cl::opt<uint32_t> TrivialPhiProcessingLimit(
29+
"mssaupdater-processing-limit", cl::Hidden, cl::init(20),
30+
cl::desc("The worklist limit for trivial phi removal (default = 20)"));
31+
2732
// This is the marker algorithm from "Simple and Efficient Construction of
2833
// Static Single Assignment Form"
2934
// The simple, non-marker algorithm places phi nodes at any join
@@ -181,18 +186,6 @@ MemoryAccess *MemorySSAUpdater::getPreviousDefFromEnd(
181186

182187
return getPreviousDefRecursive(BB, CachedPreviousDef);
183188
}
184-
// Recurse over a set of phi uses to eliminate the trivial ones
185-
MemoryAccess *MemorySSAUpdater::recursePhi(MemoryAccess *Phi) {
186-
if (!Phi)
187-
return nullptr;
188-
TrackingVH<MemoryAccess> Res(Phi);
189-
SmallVector<TrackingVH<Value>, 8> Uses;
190-
std::copy(Phi->user_begin(), Phi->user_end(), std::back_inserter(Uses));
191-
for (auto &U : Uses)
192-
if (MemoryPhi *UsePhi = dyn_cast<MemoryPhi>(&*U))
193-
tryRemoveTrivialPhi(UsePhi);
194-
return Res;
195-
}
196189

197190
// Eliminate trivial phis
198191
// Phis are trivial if they are defined either by themselves, or all the same
@@ -230,9 +223,61 @@ MemoryAccess *MemorySSAUpdater::tryRemoveTrivialPhi(MemoryPhi *Phi,
230223
removeMemoryAccess(Phi);
231224
}
232225

233-
// We should only end up recursing in case we replaced something, in which
234-
// case, we may have made other Phis trivial.
235-
return recursePhi(Same);
226+
// Continue traversal in a DFS worklist approach, in case we might find
227+
// other trivial Phis.
228+
if (!Same)
229+
return nullptr;
230+
231+
TrackingVH<MemoryAccess> Result(Same);
232+
233+
// Worklist approach to recursively removing trivial Phis.
234+
SmallVector<TrackingVH<Value>, 5> Worklist;
235+
236+
for (auto U : Same->users()) {
237+
if (dyn_cast<MemoryPhi>(&*U))
238+
Worklist.push_back(TrackingVH<Value>(U));
239+
}
240+
241+
while (!Worklist.empty() || Worklist.size() > TrivialPhiProcessingLimit) {
242+
MemoryPhi *RecPhi = dyn_cast<MemoryPhi>(&*(Worklist[Worklist.size() - 1]));
243+
Worklist.pop_back();
244+
245+
if (!RecPhi)
246+
continue;
247+
auto RecOperands = RecPhi->operands();
248+
249+
// Repeat above algorithm.
250+
if (NonOptPhis.count(RecPhi))
251+
continue;
252+
253+
bool nextone = false;
254+
MemoryAccess *RecSame = nullptr;
255+
for (auto &Op : RecOperands) {
256+
if (Op == RecPhi || Op == RecSame)
257+
continue;
258+
if (RecSame) {
259+
nextone = true;
260+
break;
261+
}
262+
RecSame = cast<MemoryAccess>(&*Op);
263+
}
264+
265+
// Move on to the next Phi in the worklist.
266+
if (nextone || RecSame == nullptr)
267+
continue;
268+
269+
if (RecPhi) {
270+
RecPhi->replaceAllUsesWith(RecSame);
271+
removeMemoryAccess(RecPhi);
272+
}
273+
274+
for (auto U : RecSame->users()) {
275+
if (dyn_cast<MemoryPhi>(&*U))
276+
Worklist.push_back(TrackingVH<Value>(U));
277+
}
278+
}
279+
280+
return Result;
236281
}
237282

238283
void MemorySSAUpdater::insertUse(MemoryUse *MU, bool RenameUses) {

0 commit comments

Comments
 (0)