[llvm] r330416 - Revert r330403 and r330413.
Ilya Biryukov via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 20 03:52:54 PDT 2018
Author: ibiryukov
Date: Fri Apr 20 03:52:54 2018
New Revision: 330416
URL: http://llvm.org/viewvc/llvm-project?rev=330416&view=rev
Log:
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:
#2 llvm::SSAUpdaterBulk::RewriteAllUses(llvm::DominatorTree*, llvm::SmallVectorImpl<llvm::PHINode*>*)
#3 llvm::JumpThreadingPass::ThreadEdge(llvm::BasicBlock*, llvm::SmallVectorImpl<llvm::BasicBlock*> const&, llvm::BasicBlock*)
#4 llvm::JumpThreadingPass::ProcessThreadableEdges(llvm::Value*, llvm::BasicBlock*, llvm::jumpthreading::ConstantPreference, llvm::Instruction*)
#5 llvm::JumpThreadingPass::ProcessBlock(llvm::BasicBlock*)
The crash happens while compiling 'lib/Analysis/CallGraph.cpp'.
r3340413 is reverted due to conflicting changes.
Removed:
llvm/trunk/test/Transforms/JumpThreading/removed-use.ll
Modified:
llvm/trunk/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
llvm/trunk/lib/Transforms/Utils/SSAUpdaterBulk.cpp
llvm/trunk/unittests/Transforms/Utils/SSAUpdaterBulk.cpp
Modified: llvm/trunk/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/SSAUpdaterBulk.h?rev=330416&r1=330415&r2=330416&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/SSAUpdaterBulk.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/SSAUpdaterBulk.h Fri Apr 20 03:52:54 2018
@@ -47,7 +47,7 @@ class SSAUpdaterBulk {
RewriteInfo(){};
RewriteInfo(StringRef &N, Type *T) : Name(N), Ty(T){};
};
- SmallVector<RewriteInfo, 4> Rewrites;
+ DenseMap<unsigned, RewriteInfo> Rewrites;
PredIteratorCache PredCache;
@@ -60,9 +60,8 @@ public:
~SSAUpdaterBulk(){};
/// Add a new variable to the SSA rewriter. This needs to be called before
- /// AddAvailableValue or AddUse calls. The return value is the variable ID,
- /// which needs to be passed to AddAvailableValue and AddUse.
- unsigned AddVariable(StringRef Name, Type *Ty);
+ /// AddAvailableValue or AddUse calls.
+ void AddVariable(unsigned Var, StringRef Name, Type *Ty);
/// Indicate that a rewritten value is available in the specified block with
/// the specified value.
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=330416&r1=330415&r2=330416&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Fri Apr 20 03:52:54 2018
@@ -66,7 +66,6 @@
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Cloning.h"
#include "llvm/Transforms/Utils/SSAUpdater.h"
-#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
#include "llvm/Transforms/Utils/ValueMapper.h"
#include <algorithm>
#include <cassert>
@@ -1990,19 +1989,15 @@ bool JumpThreadingPass::ThreadEdge(Basic
// now have to update all uses of the value to use either the original value,
// the cloned value, or some PHI derived value. This can require arbitrary
// PHI insertion, of which we are prepared to do, clean these up now.
- SSAUpdaterBulk SSAUpdate;
-
+ SSAUpdater SSAUpdate;
+ SmallVector<Use*, 16> UsesToRename;
for (Instruction &I : *BB) {
- SmallVector<Use*, 16> UsesToRename;
-
// Scan all uses of this instruction to see if it is used outside of its
- // block, and if so, record them in UsesToRename. Also, skip phi operands
- // from PredBB - we'll remove them anyway.
+ // block, and if so, record them in UsesToRename.
for (Use &U : I.uses()) {
Instruction *User = cast<Instruction>(U.getUser());
if (PHINode *UserPN = dyn_cast<PHINode>(User)) {
- if (UserPN->getIncomingBlock(U) == BB ||
- UserPN->getIncomingBlock(U) == PredBB)
+ if (UserPN->getIncomingBlock(U) == BB)
continue;
} else if (User->getParent() == BB)
continue;
@@ -2013,14 +2008,19 @@ bool JumpThreadingPass::ThreadEdge(Basic
// If there are no uses outside the block, we're done with this instruction.
if (UsesToRename.empty())
continue;
- unsigned VarNum = SSAUpdate.AddVariable(I.getName(), I.getType());
- // We found a use of I outside of BB - we need to rename all uses of I that
- // are outside its block to be uses of the appropriate PHI node etc.
- SSAUpdate.AddAvailableValue(VarNum, BB, &I);
- SSAUpdate.AddAvailableValue(VarNum, NewBB, ValueMapping[&I]);
- for (auto *U : UsesToRename)
- SSAUpdate.AddUse(VarNum, U);
+ DEBUG(dbgs() << "JT: Renaming non-local uses of: " << I << "\n");
+
+ // We found a use of I outside of BB. Rename all uses of I that are outside
+ // its block to be uses of the appropriate PHI node etc. See ValuesInBlocks
+ // with the two values we know.
+ SSAUpdate.Initialize(I.getType(), I.getName());
+ SSAUpdate.AddAvailableValue(BB, &I);
+ SSAUpdate.AddAvailableValue(NewBB, ValueMapping[&I]);
+
+ while (!UsesToRename.empty())
+ SSAUpdate.RewriteUse(*UsesToRename.pop_back_val());
+ DEBUG(dbgs() << "\n");
}
// Ok, NewBB is good to go. Update the terminator of PredBB to jump to
@@ -2037,10 +2037,6 @@ bool JumpThreadingPass::ThreadEdge(Basic
{DominatorTree::Insert, PredBB, NewBB},
{DominatorTree::Delete, PredBB, BB}});
- // Apply all updates we queued with DDT and get the updated Dominator Tree.
- DominatorTree *DT = &DDT->flush();
- SSAUpdate.RewriteAllUses(DT);
-
// At this point, the IR is fully up to date and consistent. Do a quick scan
// over the new instructions and zap any that are constants or dead. This
// frequently happens because of phi translation.
Modified: llvm/trunk/lib/Transforms/Utils/SSAUpdaterBulk.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SSAUpdaterBulk.cpp?rev=330416&r1=330415&r2=330416&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SSAUpdaterBulk.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SSAUpdaterBulk.cpp Fri Apr 20 03:52:54 2018
@@ -38,19 +38,18 @@ static BasicBlock *getUserBB(Use *U) {
/// Add a new variable to the SSA rewriter. This needs to be called before
/// AddAvailableValue or AddUse calls.
-unsigned SSAUpdaterBulk::AddVariable(StringRef Name, Type *Ty) {
- unsigned Var = Rewrites.size();
+void SSAUpdaterBulk::AddVariable(unsigned Var, StringRef Name, Type *Ty) {
+ assert(Rewrites.find(Var) == Rewrites.end() && "Variable added twice!");
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": initialized with Ty = " << *Ty
<< ", Name = " << Name << "\n");
RewriteInfo RI(Name, Ty);
- Rewrites.push_back(RI);
- return Var;
+ Rewrites[Var] = RI;
}
/// Indicate that a rewritten value is available in the specified block with the
/// specified value.
void SSAUpdaterBulk::AddAvailableValue(unsigned Var, BasicBlock *BB, Value *V) {
- assert(Var < Rewrites.size() && "Variable not found!");
+ assert(Rewrites.find(Var) != Rewrites.end() && "Should add variable first!");
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": added new available value"
<< *V << " in " << BB->getName() << "\n");
Rewrites[Var].Defines[BB] = V;
@@ -59,7 +58,7 @@ void SSAUpdaterBulk::AddAvailableValue(u
/// Record a use of the symbolic value. This use will be updated with a
/// rewritten value when RewriteAllUses is called.
void SSAUpdaterBulk::AddUse(unsigned Var, Use *U) {
- assert(Var < Rewrites.size() && "Variable not found!");
+ assert(Rewrites.find(Var) != Rewrites.end() && "Should add variable first!");
DEBUG(dbgs() << "SSAUpdater: Var=" << Var << ": added a use" << *U->get()
<< " in " << getUserBB(U)->getName() << "\n");
Rewrites[Var].Uses.push_back(U);
@@ -68,7 +67,7 @@ void SSAUpdaterBulk::AddUse(unsigned Var
/// Return true if the SSAUpdater already has a value for the specified variable
/// in the specified block.
bool SSAUpdaterBulk::HasValueForBlock(unsigned Var, BasicBlock *BB) {
- return (Var < Rewrites.size()) ? Rewrites[Var].Defines.count(BB) : false;
+ return Rewrites.count(Var) ? Rewrites[Var].Defines.count(BB) : false;
}
// Compute value at the given block BB. We either should already know it, or we
@@ -127,14 +126,16 @@ ComputeLiveInBlocks(const SmallPtrSetImp
/// requested uses update.
void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
SmallVectorImpl<PHINode *> *InsertedPHIs) {
- for (auto &R : Rewrites) {
+ for (auto &P : Rewrites) {
// Compute locations for new phi-nodes.
// For that we need to initialize DefBlocks from definitions in R.Defines,
// UsingBlocks from uses in R.Uses, then compute LiveInBlocks, and then use
// this set for computing iterated dominance frontier (IDF).
// The IDF blocks are the blocks where we need to insert new phi-nodes.
ForwardIDFCalculator IDF(*DT);
- DEBUG(dbgs() << "SSAUpdater: rewriting " << R.Uses.size() << " use(s)\n");
+ RewriteInfo &R = P.second;
+ DEBUG(dbgs() << "SSAUpdater: Var=" << P.first << ": rewriting "
+ << R.Uses.size() << " use(s)\n");
SmallPtrSet<BasicBlock *, 2> DefBlocks;
for (auto &Def : R.Defines)
@@ -164,7 +165,7 @@ void SSAUpdaterBulk::RewriteAllUses(Domi
}
// Fill in arguments of the inserted PHIs.
- for (auto *PN : InsertedPHIsForVar) {
+ for (auto PN : InsertedPHIsForVar) {
BasicBlock *PBB = PN->getParent();
for (BasicBlock *Pred : PredCache.get(PBB))
PN->addIncoming(computeValueAt(Pred, R, DT), Pred);
@@ -181,8 +182,8 @@ void SSAUpdaterBulk::RewriteAllUses(Domi
// Notify that users of the existing value that it is being replaced.
if (OldVal != V && OldVal->hasValueHandle())
ValueHandleBase::ValueIsRAUWd(OldVal, V);
- DEBUG(dbgs() << "SSAUpdater: replacing " << *OldVal << " with " << *V
- << "\n");
+ DEBUG(dbgs() << "SSAUpdater: Var=" << P.first << ": replacing" << *OldVal
+ << " with " << *V << "\n");
U->set(V);
}
}
Removed: llvm/trunk/test/Transforms/JumpThreading/removed-use.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/removed-use.ll?rev=330415&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/removed-use.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/removed-use.ll (removed)
@@ -1,28 +0,0 @@
-; RUN: opt -S < %s -jump-threading | FileCheck %s
-; CHECK-LABEL: @foo
-; CHECK: bb6:
-; CHECK-NEXT: ret void
-; CHECK: bb3:
-; CHECK: br label %bb3
-define void @foo() {
-entry:
- br i1 true, label %bb6, label %bb3
-
-bb3:
- %x0 = phi i32 [ undef, %entry ], [ %x1, %bb5 ]
- %y = and i64 undef, 1
- %p = icmp ne i64 %y, 0
- br i1 %p, label %bb4, label %bb5
-
-bb4:
- br label %bb5
-
-bb5:
- %x1 = phi i32 [ %x0, %bb3 ], [ %x0, %bb4 ]
- %z = phi i32 [ 0, %bb3 ], [ 1, %bb4 ]
- %q = icmp eq i32 %z, 0
- br i1 %q, label %bb3, label %bb6
-
-bb6:
- ret void
-}
Modified: llvm/trunk/unittests/Transforms/Utils/SSAUpdaterBulk.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/SSAUpdaterBulk.cpp?rev=330416&r1=330415&r2=330416&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/SSAUpdaterBulk.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/SSAUpdaterBulk.cpp Fri Apr 20 03:52:54 2018
@@ -73,17 +73,17 @@ TEST(SSAUpdaterBulk, SimpleMerge) {
// SSAUpdater should insert into %merge.
// Intentionally don't touch %8 to see that SSAUpdater only changes
// instructions that were explicitly specified.
- unsigned VarNum = Updater.AddVariable("a", I32Ty);
- Updater.AddAvailableValue(VarNum, TrueBB, AddOp1);
- Updater.AddAvailableValue(VarNum, FalseBB, AddOp2);
- Updater.AddUse(VarNum, &I1->getOperandUse(0));
- Updater.AddUse(VarNum, &I2->getOperandUse(0));
-
- VarNum = Updater.AddVariable("b", I32Ty);
- Updater.AddAvailableValue(VarNum, TrueBB, SubOp1);
- Updater.AddAvailableValue(VarNum, FalseBB, SubOp2);
- Updater.AddUse(VarNum, &I3->getOperandUse(0));
- Updater.AddUse(VarNum, &I3->getOperandUse(1));
+ Updater.AddVariable(0, "a", I32Ty);
+ Updater.AddAvailableValue(0, TrueBB, AddOp1);
+ Updater.AddAvailableValue(0, FalseBB, AddOp2);
+ Updater.AddUse(0, &I1->getOperandUse(0));
+ Updater.AddUse(0, &I2->getOperandUse(0));
+
+ Updater.AddVariable(1, "b", I32Ty);
+ Updater.AddAvailableValue(1, TrueBB, SubOp1);
+ Updater.AddAvailableValue(1, FalseBB, SubOp2);
+ Updater.AddUse(1, &I3->getOperandUse(0));
+ Updater.AddUse(1, &I3->getOperandUse(1));
DominatorTree DT(*F);
Updater.RewriteAllUses(&DT);
@@ -161,19 +161,19 @@ TEST(SSAUpdaterBulk, Irreducible) {
// No other rewrites should be made.
// Add use in %3.
- unsigned VarNum = Updater.AddVariable("c", I32Ty);
- Updater.AddAvailableValue(VarNum, IfBB, AddOp1);
- Updater.AddUse(VarNum, &I1->getOperandUse(0));
+ Updater.AddVariable(0, "c", I32Ty);
+ Updater.AddAvailableValue(0, IfBB, AddOp1);
+ Updater.AddUse(0, &I1->getOperandUse(0));
// Add use in %4.
- VarNum = Updater.AddVariable("b", I32Ty);
- Updater.AddAvailableValue(VarNum, LoopStartBB, AddOp2);
- Updater.AddUse(VarNum, &I2->getOperandUse(0));
+ Updater.AddVariable(1, "b", I32Ty);
+ Updater.AddAvailableValue(1, LoopStartBB, AddOp2);
+ Updater.AddUse(1, &I2->getOperandUse(0));
// Add use in the return instruction.
- VarNum = Updater.AddVariable("a", I32Ty);
- Updater.AddAvailableValue(VarNum, &F->getEntryBlock(), FirstArg);
- Updater.AddUse(VarNum, &Return->getOperandUse(0));
+ Updater.AddVariable(2, "a", I32Ty);
+ Updater.AddAvailableValue(2, &F->getEntryBlock(), FirstArg);
+ Updater.AddUse(2, &Return->getOperandUse(0));
// Save all inserted phis into a vector.
SmallVector<PHINode *, 8> Inserted;
More information about the llvm-commits
mailing list