[llvm] r332168 - Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading."

Michael Zolotukhin via llvm-commits llvm-commits at lists.llvm.org
Fri May 11 18:52:36 PDT 2018


Author: mzolotukhin
Date: Fri May 11 18:52:36 2018
New Revision: 332168

URL: http://llvm.org/viewvc/llvm-project?rev=332168&view=rev
Log:
Reapply "[PR16756] Use SSAUpdaterBulk in JumpThreading."

Stage3/stage4 bootstrap miscompares should be fixed by a non-determinism
fix in IDF (r332167).

This reverts commit r330446.

Modified:
    llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=332168&r1=332167&r2=332168&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Fri May 11 18:52:36 2018
@@ -66,6 +66,7 @@
 #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>
@@ -1996,19 +1997,33 @@ bool JumpThreadingPass::ThreadEdge(Basic
   // PHI nodes for NewBB now.
   AddPHINodeEntriesForMappedBlock(SuccBB, BB, NewBB, ValueMapping);
 
+  // Update the terminator of PredBB to jump to NewBB instead of BB.  This
+  // eliminates predecessors from BB, which requires us to simplify any PHI
+  // nodes in BB.
+  TerminatorInst *PredTerm = PredBB->getTerminator();
+  for (unsigned i = 0, e = PredTerm->getNumSuccessors(); i != e; ++i)
+    if (PredTerm->getSuccessor(i) == BB) {
+      BB->removePredecessor(PredBB, true);
+      PredTerm->setSuccessor(i, NewBB);
+    }
+
   // If there were values defined in BB that are used outside the block, then we
   // 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.
-  SSAUpdater SSAUpdate;
-  SmallVector<Use*, 16> UsesToRename;
+  SSAUpdaterBulk SSAUpdate;
+
   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.
+    // block, and if so, record them in UsesToRename. Also, skip phi operands
+    // from PredBB - we'll remove them anyway.
     for (Use &U : I.uses()) {
       Instruction *User = cast<Instruction>(U.getUser());
       if (PHINode *UserPN = dyn_cast<PHINode>(User)) {
-        if (UserPN->getIncomingBlock(U) == BB)
+        if (UserPN->getIncomingBlock(U) == BB ||
+            UserPN->getIncomingBlock(U) == PredBB)
           continue;
       } else if (User->getParent() == BB)
         continue;
@@ -2019,35 +2034,24 @@ 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());
 
-    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
-  // NewBB instead of BB.  This eliminates predecessors from BB, which requires
-  // us to simplify any PHI nodes in BB.
-  TerminatorInst *PredTerm = PredBB->getTerminator();
-  for (unsigned i = 0, e = PredTerm->getNumSuccessors(); i != e; ++i)
-    if (PredTerm->getSuccessor(i) == BB) {
-      BB->removePredecessor(PredBB, true);
-      PredTerm->setSuccessor(i, NewBB);
-    }
+    // 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);
+  }
 
   DDT->applyUpdates({{DominatorTree::Insert, NewBB, SuccBB},
                      {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.




More information about the llvm-commits mailing list