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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@330416 91177308-0d34-0410-b5e6-96231b3b80d8

Author: ilya-biryukov

include/llvm/Transforms/Utils/SSAUpdaterBulk.h

@@ -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 @@ class SSAUpdaterBulk {
   ~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.

lib/Transforms/Scalar/JumpThreading.cpp

@@ -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(BasicBlock *BB,
   // 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(BasicBlock *BB,
     // 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(BasicBlock *BB,
                      {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.

lib/Transforms/Utils/SSAUpdaterBulk.cpp

@@ -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(unsigned Var, BasicBlock *BB, Value *V) {
 /// 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, Use *U) {
 /// 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 SmallPtrSetImpl<BasicBlock *> &UsingBlocks,
 /// 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(DominatorTree *DT,
     }
 
     // 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(DominatorTree *DT,
       // 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);
     }
   }

test/Transforms/JumpThreading/removed-use.ll

@@ -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;