[PATCH] D51471: [SimplifyCFG] Reduce memory churn during common case for branch commoning [NFC]

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 163239.
reames added a comment.

reverse last rebase, the small addition was buggy (iterator invalidation)


https://reviews.llvm.org/D51471

Files:
  lib/Transforms/Utils/SimplifyCFG.cpp


Index: lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyCFG.cpp
+++ lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2686,40 +2686,51 @@
       PBI->swapSuccessors();
     }
 
-    // If we have bonus instructions, clone them into the predecessor block.
-    // Note that there may be multiple predecessor blocks, so we cannot move
-    // bonus instructions to a predecessor block.
-    ValueToValueMapTy VMap; // maps original values to cloned values
-    // We already make sure Cond is the last instruction before BI. Therefore,
-    // all instructions before Cond other than DbgInfoIntrinsic are bonus
-    // instructions.
-    for (auto BonusInst = BB->begin(); Cond != &*BonusInst; ++BonusInst) {
-      Instruction *NewBonusInst = BonusInst->clone();
-      RemapInstruction(NewBonusInst, VMap,
+    // Ensure all instructions are cloned to the predecessor block, and
+    // identify the copy of Cond in the predecessor.  As a small optimization
+    // for the common case to reduce allocation traffic, we move instructions
+    // if we know there is at most a single predecessor.
+    Instruction *CondInPred = nullptr;
+    if (PredCount == 1) {
+      for (auto BonusInstItr = BB->begin(); Cond != &*BonusInstItr;) {
+        auto BonusInst = BonusInstItr++;
+        BonusInst->moveBefore(PBI);
+        // Remove all potentially control dependent facts (TODO: could be more
+        // fine grained here)
+        BonusInst->dropUnknownNonDebugMetadata();
+      }
+      CondInPred = Cond;
+      CondInPred->moveBefore(PBI);
+    } else {
+      // Handle the general case where we may have more than one predecessor
+      // and thus need to clone rather than move.  
+      ValueToValueMapTy VMap; // maps original values to cloned values
+      // We already make sure Cond is the last instruction before BI. Therefore,
+      // all instructions before Cond other than DbgInfoIntrinsic are bonus
+      // instructions.
+      for (auto BonusInst = BB->begin(); Cond != &*BonusInst; ++BonusInst) {
+        Instruction *NewBonusInst = BonusInst->clone();
+        RemapInstruction(NewBonusInst, VMap,
+                         RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+        VMap[&*BonusInst] = NewBonusInst;
+        
+        // Remove all potentially control dependent facts (TODO: could be more
+        // fine grained here)
+        BonusInst->dropUnknownNonDebugMetadata();
+        
+        PredBlock->getInstList().insert(PBI->getIterator(), NewBonusInst);
+        NewBonusInst->takeName(&*BonusInst);
+        BonusInst->setName(BonusInst->getName() + ".old");
+      }
+
+      // Clone Cond into the predecessor basic block
+      CondInPred = Cond->clone();
+      RemapInstruction(CondInPred, VMap,
                        RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
-      VMap[&*BonusInst] = NewBonusInst;
-
-      // If we moved a load, we cannot any longer claim any knowledge about
-      // its potential value. The previous information might have been valid
-      // only given the branch precondition.
-      // For an analogous reason, we must also drop all the metadata whose
-      // semantics we don't understand.
-      NewBonusInst->dropUnknownNonDebugMetadata();
-
-      PredBlock->getInstList().insert(PBI->getIterator(), NewBonusInst);
-      NewBonusInst->takeName(&*BonusInst);
-      BonusInst->setName(BonusInst->getName() + ".old");
-    }
-
-    // Clone Cond into the predecessor basic block, and or/and the
-    // two conditions together.
-    Instruction *CondInPred = Cond->clone();
-    RemapInstruction(CondInPred, VMap,
-                     RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
-    PredBlock->getInstList().insert(PBI->getIterator(), CondInPred);
-    CondInPred->takeName(Cond);
-    Cond->setName(CondInPred->getName() + ".old");
-
+      PredBlock->getInstList().insert(PBI->getIterator(), CondInPred);
+      CondInPred->takeName(Cond);
+      Cond->setName(CondInPred->getName() + ".old");
+    }
     if (BI->isConditional()) {
       Instruction *NewCond = cast<Instruction>(
           Builder.CreateBinOp(Opc, PBI->getCondition(), CondInPred, "or.cond"));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51471.163239.patch
Type: text/x-patch
Size: 4256 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180830/24a1781f/attachment.bin>