[llvm] 60dd012 - Revert "[SimplifyCFG] performBranchToCommonDestFolding(): form block-closed SSA form before cloning instructions (PR51125)"

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 15 09:16:35 PDT 2021


Author: Roman Lebedev
Date: 2021-08-15T19:15:09+03:00
New Revision: 60dd0121c92e93a33cf39b5f7006923ac9e4f127

URL: https://github.com/llvm/llvm-project/commit/60dd0121c92e93a33cf39b5f7006923ac9e4f127
DIFF: https://github.com/llvm/llvm-project/commit/60dd0121c92e93a33cf39b5f7006923ac9e4f127.diff

LOG: Revert "[SimplifyCFG] performBranchToCommonDestFolding(): form block-closed SSA form before cloning instructions (PR51125)"

Forgot to stage the test change.

This reverts commit 78af5cb213b2f9fe3f47bf23947f14ac07024155.

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 68a0388398fc..847fdd760d2f 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1095,24 +1095,17 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
 
     // Update (liveout) uses of bonus instructions,
     // now that the bonus instruction has been cloned into predecessor.
-    // Note that we expect to be in a block-closed SSA form for this to work!
+    SSAUpdater SSAUpdate;
+    SSAUpdate.Initialize(BonusInst.getType(),
+                         (NewBonusInst->getName() + ".merge").str());
+    SSAUpdate.AddAvailableValue(BB, &BonusInst);
+    SSAUpdate.AddAvailableValue(PredBlock, NewBonusInst);
     for (Use &U : make_early_inc_range(BonusInst.uses())) {
       auto *UI = cast<Instruction>(U.getUser());
-      auto *PN = dyn_cast<PHINode>(UI);
-      if (!PN) {
-        assert(UI->getParent() == BB && BonusInst.comesBefore(UI) &&
-               "If the user is not a PHI node, then it should be in the same "
-               "block as, and come after, the original bonus instruction.");
-        continue; // Keep using the original bonus instruction.
-      }
-      // Is this the block-closed SSA form PHI node?
-      if (PN->getIncomingBlock(U) == BB)
-        continue; // Great, keep using the original bonus instruction.
-      // The only other alternative is an "use" when coming from
-      // the predecessor block - here we should refer to the cloned bonus instr.
-      assert(PN->getIncomingBlock(U) == PredBlock &&
-             "Not in block-closed SSA form?");
-      U.set(NewBonusInst);
+      if (UI->getParent() != PredBlock)
+        SSAUpdate.RewriteUseAfterInsertions(U);
+      else // Use is in the same block as, and comes before, NewBonusInst.
+        SSAUpdate.RewriteUse(U);
     }
   }
 }
@@ -3039,56 +3032,6 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
 
   LLVM_DEBUG(dbgs() << "FOLDING BRANCH TO COMMON DEST:\n" << *PBI << *BB);
 
-  // We want to duplicate all the bonus instructions in this block,
-  // and rewrite their uses, but in some cases with self-loops,
-  // the naive use rewrite approach won't work (will result in miscompilations).
-  // To avoid this problem, let's form block-closed SSA form.
-  for (Instruction &BonusInst :
-       reverse(iterator_range<BasicBlock::iterator>(*BB))) {
-    auto IsBCSSAUse = [BB, &BonusInst](Use &U) {
-      auto *UI = cast<Instruction>(U.getUser());
-      if (auto *PN = dyn_cast<PHINode>(UI))
-        return PN->getIncomingBlock(U) == BB;
-      return UI->getParent() == BB && BonusInst.comesBefore(UI);
-    };
-
-    // Does this instruction require rewriting of uses?
-    if (all_of(BonusInst.uses(), IsBCSSAUse))
-      continue;
-
-    SSAUpdater SSAUpdate;
-    Type *Ty = BonusInst.getType();
-    SmallVector<PHINode *, 8> BCSSAPHIs;
-    SSAUpdate.Initialize(Ty, BonusInst.getName());
-
-    // Into each successor block of BB, insert a PHI node, that receives
-    // the BonusInst when coming from it's basic block, or poison otherwise.
-    for (BasicBlock *Succ : successors(BB)) {
-      // The block may have the same successor multiple times. Do it only once.
-      if (SSAUpdate.HasValueForBlock(Succ))
-        continue;
-      BCSSAPHIs.emplace_back(PHINode::Create(
-          Ty, 0, BonusInst.getName() + ".bcssa", &Succ->front()));
-      PHINode *PN = BCSSAPHIs.back();
-      for (BasicBlock *PredOfSucc : predecessors(Succ))
-        PN->addIncoming(PredOfSucc == BB ? (Value *)&BonusInst
-                                         : PoisonValue::get(Ty),
-                        PredOfSucc);
-      SSAUpdate.AddAvailableValue(Succ, PN);
-    }
-
-    // And rewrite all uses that break block-closed SSA form.
-    for (Use &U : make_early_inc_range(BonusInst.uses()))
-      if (!IsBCSSAUse(U))
-        SSAUpdate.RewriteUseAfterInsertions(U);
-
-    // We might not have ended up needing PHI's in all of the succ blocks,
-    // drop the ones that are certainly unused, but don't bother otherwise.
-    for (PHINode *PN : BCSSAPHIs)
-      if (PN->use_empty())
-        PN->eraseFromParent();
-  }
-
   IRBuilder<> Builder(PBI);
   // The builder is used to create instructions to eliminate the branch in BB.
   // If BB's terminator has !annotation metadata, add it to the new


        


More information about the llvm-commits mailing list