Skip to content

Commit 8e71a93

Browse files
committed
Revert r330403 and r330413.
Revert r330413: "[SSAUpdaterBulk] Use SmallVector instead of DenseMap for storing rewrites." Revert r330403 "Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading." one more time." r330403 commit seems to crash clang during our integrate while doing PGO build with the following stacktrace: brson#2 llvm::SSAUpdaterBulk::RewriteAllUses(llvm::DominatorTree*, llvm::SmallVectorImpl<llvm::PHINode*>*) brson#3 llvm::JumpThreadingPass::ThreadEdge(llvm::BasicBlock*, llvm::SmallVectorImpl<llvm::BasicBlock*> const&, llvm::BasicBlock*) brson#4 llvm::JumpThreadingPass::ProcessThreadableEdges(llvm::Value*, llvm::BasicBlock*, llvm::jumpthreading::ConstantPreference, llvm::Instruction*) brson#5 llvm::JumpThreadingPass::ProcessBlock(llvm::BasicBlock*) The crash happens while compiling 'lib/Analysis/CallGraph.cpp'. r3340413 is reverted due to conflicting changes. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@330416 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent ef1b331 commit 8e71a93

File tree

5 files changed

+52
-84
lines changed

5 files changed

+52
-84
lines changed

include/llvm/Transforms/Utils/SSAUpdaterBulk.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class SSAUpdaterBulk {
4747
RewriteInfo(){};
4848
RewriteInfo(StringRef &N, Type *T) : Name(N), Ty(T){};
4949
};
50-
SmallVector<RewriteInfo, 4> Rewrites;
50+
DenseMap<unsigned, RewriteInfo> Rewrites;
5151

5252
PredIteratorCache PredCache;
5353

@@ -60,9 +60,8 @@ class SSAUpdaterBulk {
6060
~SSAUpdaterBulk(){};
6161

6262
/// Add a new variable to the SSA rewriter. This needs to be called before
63-
/// AddAvailableValue or AddUse calls. The return value is the variable ID,
64-
/// which needs to be passed to AddAvailableValue and AddUse.
65-
unsigned AddVariable(StringRef Name, Type *Ty);
63+
/// AddAvailableValue or AddUse calls.
64+
void AddVariable(unsigned Var, StringRef Name, Type *Ty);
6665

6766
/// Indicate that a rewritten value is available in the specified block with
6867
/// the specified value.

lib/Transforms/Scalar/JumpThreading.cpp

+16-20
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
6767
#include "llvm/Transforms/Utils/Cloning.h"
6868
#include "llvm/Transforms/Utils/SSAUpdater.h"
69-
#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
7069
#include "llvm/Transforms/Utils/ValueMapper.h"
7170
#include <algorithm>
7271
#include <cassert>
@@ -1990,19 +1989,15 @@ bool JumpThreadingPass::ThreadEdge(BasicBlock *BB,
19901989
// now have to update all uses of the value to use either the original value,
19911990
// the cloned value, or some PHI derived value. This can require arbitrary
19921991
// PHI insertion, of which we are prepared to do, clean these up now.
1993-
SSAUpdaterBulk SSAUpdate;
1994-
1992+
SSAUpdater SSAUpdate;
1993+
SmallVector<Use*, 16> UsesToRename;
19951994
for (Instruction &I : *BB) {
1996-
SmallVector<Use*, 16> UsesToRename;
1997-
19981995
// Scan all uses of this instruction to see if it is used outside of its
1999-
// block, and if so, record them in UsesToRename. Also, skip phi operands
2000-
// from PredBB - we'll remove them anyway.
1996+
// block, and if so, record them in UsesToRename.
20011997
for (Use &U : I.uses()) {
20021998
Instruction *User = cast<Instruction>(U.getUser());
20031999
if (PHINode *UserPN = dyn_cast<PHINode>(User)) {
2004-
if (UserPN->getIncomingBlock(U) == BB ||
2005-
UserPN->getIncomingBlock(U) == PredBB)
2000+
if (UserPN->getIncomingBlock(U) == BB)
20062001
continue;
20072002
} else if (User->getParent() == BB)
20082003
continue;
@@ -2013,14 +2008,19 @@ bool JumpThreadingPass::ThreadEdge(BasicBlock *BB,
20132008
// If there are no uses outside the block, we're done with this instruction.
20142009
if (UsesToRename.empty())
20152010
continue;
2016-
unsigned VarNum = SSAUpdate.AddVariable(I.getName(), I.getType());
20172011

2018-
// We found a use of I outside of BB - we need to rename all uses of I that
2019-
// are outside its block to be uses of the appropriate PHI node etc.
2020-
SSAUpdate.AddAvailableValue(VarNum, BB, &I);
2021-
SSAUpdate.AddAvailableValue(VarNum, NewBB, ValueMapping[&I]);
2022-
for (auto *U : UsesToRename)
2023-
SSAUpdate.AddUse(VarNum, U);
2012+
DEBUG(dbgs() << "JT: Renaming non-local uses of: " << I << "\n");
2013+
2014+
// We found a use of I outside of BB. Rename all uses of I that are outside
2015+
// its block to be uses of the appropriate PHI node etc. See ValuesInBlocks
2016+
// with the two values we know.
2017+
SSAUpdate.Initialize(I.getType(), I.getName());
2018+
SSAUpdate.AddAvailableValue(BB, &I);
2019+
SSAUpdate.AddAvailableValue(NewBB, ValueMapping[&I]);
2020+
2021+
while (!UsesToRename.empty())
2022+
SSAUpdate.RewriteUse(*UsesToRename.pop_back_val());
2023+
DEBUG(dbgs() << "\n");
20242024
}
20252025

20262026
// Ok, NewBB is good to go. Update the terminator of PredBB to jump to
@@ -2037,10 +2037,6 @@ bool JumpThreadingPass::ThreadEdge(BasicBlock *BB,
20372037
{DominatorTree::Insert, PredBB, NewBB},
20382038
{DominatorTree::Delete, PredBB, BB}});
20392039

2040-
// Apply all updates we queued with DDT and get the updated Dominator Tree.
2041-
DominatorTree *DT = &DDT->flush();
2042-
SSAUpdate.RewriteAllUses(DT);
2043-
20442040
// At this point, the IR is fully up to date and consistent. Do a quick scan
20452041
// over the new instructions and zap any that are constants or dead. This
20462042
// frequently happens because of phi translation.

lib/Transforms/Utils/SSAUpdaterBulk.cpp

+13-12
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,18 @@ static BasicBlock *getUserBB(Use *U) {
3838

3939
/// Add a new variable to the SSA rewriter. This needs to be called before
4040
/// AddAvailableValue or AddUse calls.
41-
unsigned SSAUpdaterBulk::AddVariable(StringRef Name, Type *Ty) {
42-
unsigned Var = Rewrites.size();
41+
void SSAUpdaterBulk::AddVariable(unsigned Var, StringRef Name, Type *Ty) {
42+
assert(Rewrites.find(Var) == Rewrites.end() && "Variable added twice!");
4343
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": initialized with Ty = " << *Ty
4444
<< ", Name = " << Name << "\n");
4545
RewriteInfo RI(Name, Ty);
46-
Rewrites.push_back(RI);
47-
return Var;
46+
Rewrites[Var] = RI;
4847
}
4948

5049
/// Indicate that a rewritten value is available in the specified block with the
5150
/// specified value.
5251
void SSAUpdaterBulk::AddAvailableValue(unsigned Var, BasicBlock *BB, Value *V) {
53-
assert(Var < Rewrites.size() && "Variable not found!");
52+
assert(Rewrites.find(Var) != Rewrites.end() && "Should add variable first!");
5453
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": added new available value"
5554
<< *V << " in " << BB->getName() << "\n");
5655
Rewrites[Var].Defines[BB] = V;
@@ -59,7 +58,7 @@ void SSAUpdaterBulk::AddAvailableValue(unsigned Var, BasicBlock *BB, Value *V) {
5958
/// Record a use of the symbolic value. This use will be updated with a
6059
/// rewritten value when RewriteAllUses is called.
6160
void SSAUpdaterBulk::AddUse(unsigned Var, Use *U) {
62-
assert(Var < Rewrites.size() && "Variable not found!");
61+
assert(Rewrites.find(Var) != Rewrites.end() && "Should add variable first!");
6362
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": added a use" << *U->get()
6463
<< " in " << getUserBB(U)->getName() << "\n");
6564
Rewrites[Var].Uses.push_back(U);
@@ -68,7 +67,7 @@ void SSAUpdaterBulk::AddUse(unsigned Var, Use *U) {
6867
/// Return true if the SSAUpdater already has a value for the specified variable
6968
/// in the specified block.
7069
bool SSAUpdaterBulk::HasValueForBlock(unsigned Var, BasicBlock *BB) {
71-
return (Var < Rewrites.size()) ? Rewrites[Var].Defines.count(BB) : false;
70+
return Rewrites.count(Var) ? Rewrites[Var].Defines.count(BB) : false;
7271
}
7372

7473
// Compute value at the given block BB. We either should already know it, or we
@@ -127,14 +126,16 @@ ComputeLiveInBlocks(const SmallPtrSetImpl<BasicBlock *> &UsingBlocks,
127126
/// requested uses update.
128127
void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
129128
SmallVectorImpl<PHINode *> *InsertedPHIs) {
130-
for (auto &R : Rewrites) {
129+
for (auto &P : Rewrites) {
131130
// Compute locations for new phi-nodes.
132131
// For that we need to initialize DefBlocks from definitions in R.Defines,
133132
// UsingBlocks from uses in R.Uses, then compute LiveInBlocks, and then use
134133
// this set for computing iterated dominance frontier (IDF).
135134
// The IDF blocks are the blocks where we need to insert new phi-nodes.
136135
ForwardIDFCalculator IDF(*DT);
137-
DEBUG(dbgs() << "SSAUpdater: rewriting " << R.Uses.size() << " use(s)\n");
136+
RewriteInfo &R = P.second;
137+
DEBUG(dbgs() << "SSAUpdater: Var=" << P.first << ": rewriting "
138+
<< R.Uses.size() << " use(s)\n");
138139

139140
SmallPtrSet<BasicBlock *, 2> DefBlocks;
140141
for (auto &Def : R.Defines)
@@ -164,7 +165,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
164165
}
165166

166167
// Fill in arguments of the inserted PHIs.
167-
for (auto *PN : InsertedPHIsForVar) {
168+
for (auto PN : InsertedPHIsForVar) {
168169
BasicBlock *PBB = PN->getParent();
169170
for (BasicBlock *Pred : PredCache.get(PBB))
170171
PN->addIncoming(computeValueAt(Pred, R, DT), Pred);
@@ -181,8 +182,8 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
181182
// Notify that users of the existing value that it is being replaced.
182183
if (OldVal != V && OldVal->hasValueHandle())
183184
ValueHandleBase::ValueIsRAUWd(OldVal, V);
184-
DEBUG(dbgs() << "SSAUpdater: replacing " << *OldVal << " with " << *V
185-
<< "\n");
185+
DEBUG(dbgs() << "SSAUpdater: Var=" << P.first << ": replacing" << *OldVal
186+
<< " with " << *V << "\n");
186187
U->set(V);
187188
}
188189
}

test/Transforms/JumpThreading/removed-use.ll

-28
This file was deleted.

unittests/Transforms/Utils/SSAUpdaterBulk.cpp

+20-20
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,17 @@ TEST(SSAUpdaterBulk, SimpleMerge) {
7373
// SSAUpdater should insert into %merge.
7474
// Intentionally don't touch %8 to see that SSAUpdater only changes
7575
// instructions that were explicitly specified.
76-
unsigned VarNum = Updater.AddVariable("a", I32Ty);
77-
Updater.AddAvailableValue(VarNum, TrueBB, AddOp1);
78-
Updater.AddAvailableValue(VarNum, FalseBB, AddOp2);
79-
Updater.AddUse(VarNum, &I1->getOperandUse(0));
80-
Updater.AddUse(VarNum, &I2->getOperandUse(0));
81-
82-
VarNum = Updater.AddVariable("b", I32Ty);
83-
Updater.AddAvailableValue(VarNum, TrueBB, SubOp1);
84-
Updater.AddAvailableValue(VarNum, FalseBB, SubOp2);
85-
Updater.AddUse(VarNum, &I3->getOperandUse(0));
86-
Updater.AddUse(VarNum, &I3->getOperandUse(1));
76+
Updater.AddVariable(0, "a", I32Ty);
77+
Updater.AddAvailableValue(0, TrueBB, AddOp1);
78+
Updater.AddAvailableValue(0, FalseBB, AddOp2);
79+
Updater.AddUse(0, &I1->getOperandUse(0));
80+
Updater.AddUse(0, &I2->getOperandUse(0));
81+
82+
Updater.AddVariable(1, "b", I32Ty);
83+
Updater.AddAvailableValue(1, TrueBB, SubOp1);
84+
Updater.AddAvailableValue(1, FalseBB, SubOp2);
85+
Updater.AddUse(1, &I3->getOperandUse(0));
86+
Updater.AddUse(1, &I3->getOperandUse(1));
8787

8888
DominatorTree DT(*F);
8989
Updater.RewriteAllUses(&DT);
@@ -161,19 +161,19 @@ TEST(SSAUpdaterBulk, Irreducible) {
161161
// No other rewrites should be made.
162162

163163
// Add use in %3.
164-
unsigned VarNum = Updater.AddVariable("c", I32Ty);
165-
Updater.AddAvailableValue(VarNum, IfBB, AddOp1);
166-
Updater.AddUse(VarNum, &I1->getOperandUse(0));
164+
Updater.AddVariable(0, "c", I32Ty);
165+
Updater.AddAvailableValue(0, IfBB, AddOp1);
166+
Updater.AddUse(0, &I1->getOperandUse(0));
167167

168168
// Add use in %4.
169-
VarNum = Updater.AddVariable("b", I32Ty);
170-
Updater.AddAvailableValue(VarNum, LoopStartBB, AddOp2);
171-
Updater.AddUse(VarNum, &I2->getOperandUse(0));
169+
Updater.AddVariable(1, "b", I32Ty);
170+
Updater.AddAvailableValue(1, LoopStartBB, AddOp2);
171+
Updater.AddUse(1, &I2->getOperandUse(0));
172172

173173
// Add use in the return instruction.
174-
VarNum = Updater.AddVariable("a", I32Ty);
175-
Updater.AddAvailableValue(VarNum, &F->getEntryBlock(), FirstArg);
176-
Updater.AddUse(VarNum, &Return->getOperandUse(0));
174+
Updater.AddVariable(2, "a", I32Ty);
175+
Updater.AddAvailableValue(2, &F->getEntryBlock(), FirstArg);
176+
Updater.AddUse(2, &Return->getOperandUse(0));
177177

178178
// Save all inserted phis into a vector.
179179
SmallVector<PHINode *, 8> Inserted;

0 commit comments

Comments
 (0)