[llvm] 751be2a - [CodeMoverUtils] Enhance isSafeToMoveBefore() when moving BBs

[via llvm-commits]

Author: Congzhe Cao
Date: 2021-09-24T05:48:15-04:00
New Revision: 751be2a064f119af74c7b9b1e52bc904d8aa114d

URL: https://github.com/llvm/llvm-project/commit/751be2a064f119af74c7b9b1e52bc904d8aa114d
DIFF: https://github.com/llvm/llvm-project/commit/751be2a064f119af74c7b9b1e52bc904d8aa114d.diff

LOG: [CodeMoverUtils] Enhance isSafeToMoveBefore() when moving BBs

When moving an entire basic block BB before InsertPoint, currently
we check for all instructions whether the operands dominates
InsertPoint, however, this can be improved such that even an
operand does not dominate InsertPoint, as long as it appears as
a previous instruction in the same BB, it is safe to move.

Reviewed By: Whitney

Differential Revision: https://reviews.llvm.org/D110378

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
    llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
    llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h b/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
index 630f936471f22..c9fd651e00606 100644
--- a/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
@@ -40,7 +40,8 @@ bool isControlFlowEquivalent(const BasicBlock &BB0, const BasicBlock &BB1,
 bool isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
                         DominatorTree &DT,
                         const PostDominatorTree *PDT = nullptr,
-                        DependenceInfo *DI = nullptr);
+                        DependenceInfo *DI = nullptr,
+                        bool CheckForEntireBlock = false);
 
 /// Return true if all instructions (except the terminator) in \p BB can be
 /// safely moved before \p InsertPoint.

diff  --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index ce982c7403aa4..8792ee2f5a224 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -309,7 +309,7 @@ collectInstructionsInBetween(Instruction &StartInst, const Instruction &EndInst,
 
 bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
                               DominatorTree &DT, const PostDominatorTree *PDT,
-                              DependenceInfo *DI) {
+                              DependenceInfo *DI, bool CheckForEntireBlock) {
   // Skip tests when we don't have PDT or DI
   if (!PDT || !DI)
     return false;
@@ -339,9 +339,17 @@ bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
           return false;
   if (!DT.dominates(&I, &InsertPoint))
     for (const Value *Op : I.operands())
-      if (auto *OpInst = dyn_cast<Instruction>(Op))
-        if (&InsertPoint == OpInst || !DT.dominates(OpInst, &InsertPoint))
+      if (auto *OpInst = dyn_cast<Instruction>(Op)) {
+        if (&InsertPoint == OpInst)
+          return false;
+        // If OpInst is an instruction that appears earlier in the same BB as
+        // I, then it is okay to move since OpInst will still be available.
+        if (CheckForEntireBlock && I.getParent() == OpInst->getParent() &&
+            DT.dominates(OpInst, &I))
+          continue;
+        if (!DT.dominates(OpInst, &InsertPoint))
           return false;
+      }
 
   DT.updateDFSNumbers();
   const bool MoveForward = domTreeLevelBefore(&DT, &I, &InsertPoint);
@@ -393,7 +401,8 @@ bool llvm::isSafeToMoveBefore(BasicBlock &BB, Instruction &InsertPoint,
     if (BB.getTerminator() == &I)
       return true;
 
-    return isSafeToMoveBefore(I, InsertPoint, DT, PDT, DI);
+    return isSafeToMoveBefore(I, InsertPoint, DT, PDT, DI,
+                              /*CheckForEntireBlock=*/true);
   });
 }
 

diff  --git a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
index 300f9c22555ae..134858c53d61e 100644
--- a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
@@ -601,6 +601,8 @@ TEST(CodeMoverUtils, IsSafeToMoveTest3) {
                    br label %for.latch
                  for.latch:
                    %cmp = icmp slt i64 %inc, %N
+                   %add = add i64 100, %N
+                   %add2 = add i64 %add, %N
                    br i1 %cmp, label %for.body, label %for.end
                  for.end:
                    ret void
@@ -611,10 +613,18 @@ TEST(CodeMoverUtils, IsSafeToMoveTest3) {
           DependenceInfo &DI) {
         Instruction *IncInst = getInstructionByName(F, "inc");
         Instruction *CmpInst = getInstructionByName(F, "cmp");
+        BasicBlock *BB0 = getBasicBlockByName(F, "for.body");
+        BasicBlock *BB1 = getBasicBlockByName(F, "for.latch");
 
         // Can move as the incoming block of %inc for %i (%for.latch) dominated
         // by %cmp.
         EXPECT_TRUE(isSafeToMoveBefore(*IncInst, *CmpInst, DT, &PDT, &DI));
+
+        // Can move as the operands of instructions in BB1 either dominate
+        // InsertPoint or appear before that instruction, e.g., %add appears
+        // before %add2 although %add does not dominate InsertPoint.
+        EXPECT_TRUE(
+            isSafeToMoveBefore(*BB1, *BB0->getTerminator(), DT, &PDT, &DI));
       });
 }