[llvm] 082e395 - [CodeMoverUtils] Make specific analysis dependent checks optional

via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 7 07:42:54 PDT 2020


Author: SharmaRithik
Date: 2020-07-07T20:11:07+05:30
New Revision: 082e3952300003ecf2eaa6bf346ae2e783b7a02e

URL: https://github.com/llvm/llvm-project/commit/082e3952300003ecf2eaa6bf346ae2e783b7a02e
DIFF: https://github.com/llvm/llvm-project/commit/082e3952300003ecf2eaa6bf346ae2e783b7a02e.diff

LOG: [CodeMoverUtils] Make specific analysis dependent checks optional

Summary: This patch makes code motion checks optional which are dependent on
specific analysis example, dominator tree, post dominator tree and dependence
info. The aim is to make the adoption of CodeMoverUtils easier for clients that
don't use analysis which were strictly required by CodeMoverUtils. This will
also help in diversifying code motion checks using other analysis example MSSA.
Authored By: RithikSharma
Reviewer: Whitney, bmahjour, etiotto
Reviewed By: Whitney
Subscribers: Prazek, hiraditya, george.burgess.iv, asbirlea, llvm-commits
Tag: LLVM
Differential Revision: https://reviews.llvm.org/D82566

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
    llvm/lib/Transforms/Scalar/LoopFuse.cpp
    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 3f5c0aed2abf..630f936471f2 100644
--- a/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/CodeMoverUtils.h
@@ -38,14 +38,16 @@ bool isControlFlowEquivalent(const BasicBlock &BB0, const BasicBlock &BB1,
 
 /// Return true if \p I can be safely moved before \p InsertPoint.
 bool isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
-                        DominatorTree &DT, const PostDominatorTree &PDT,
-                        DependenceInfo &DI);
+                        DominatorTree &DT,
+                        const PostDominatorTree *PDT = nullptr,
+                        DependenceInfo *DI = nullptr);
 
 /// Return true if all instructions (except the terminator) in \p BB can be
 /// safely moved before \p InsertPoint.
 bool isSafeToMoveBefore(BasicBlock &BB, Instruction &InsertPoint,
-                        DominatorTree &DT, const PostDominatorTree &PDT,
-                        DependenceInfo &DI);
+                        DominatorTree &DT,
+                        const PostDominatorTree *PDT = nullptr,
+                        DependenceInfo *DI = nullptr);
 
 /// Move instructions, in an order-preserving manner, from \p FromBB to the
 /// beginning of \p ToBB when proven safe.

diff  --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
index e29bf6325850..20edc8699d79 100644
--- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp
@@ -743,8 +743,8 @@ struct LoopFuser {
           }
 
           if (!isSafeToMoveBefore(*FC1->Preheader,
-                                  *FC0->Preheader->getTerminator(), DT, PDT,
-                                  DI)) {
+                                  *FC0->Preheader->getTerminator(), DT, &PDT,
+                                  &DI)) {
             LLVM_DEBUG(dbgs() << "Fusion candidate contains unsafe "
                                  "instructions in preheader. Not fusing.\n");
             reportLoopFusion<OptimizationRemarkMissed>(*FC0, *FC1,
@@ -757,7 +757,7 @@ struct LoopFuser {
 
             if (!isSafeToMoveBefore(*FC0->ExitBlock,
                                     *FC1->ExitBlock->getFirstNonPHIOrDbg(), DT,
-                                    PDT, DI)) {
+                                    &PDT, &DI)) {
               LLVM_DEBUG(dbgs() << "Fusion candidate contains unsafe "
                                    "instructions in exit block. Not fusing.\n");
               reportLoopFusion<OptimizationRemarkMissed>(*FC0, *FC1,
@@ -767,8 +767,8 @@ struct LoopFuser {
 
             if (!isSafeToMoveBefore(
                     *FC1->GuardBranch->getParent(),
-                    *FC0->GuardBranch->getParent()->getTerminator(), DT, PDT,
-                    DI)) {
+                    *FC0->GuardBranch->getParent()->getTerminator(), DT, &PDT,
+                    &DI)) {
               LLVM_DEBUG(dbgs()
                          << "Fusion candidate contains unsafe "
                             "instructions in guard block. Not fusing.\n");

diff  --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index 4583ff74167a..11a740f8285b 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -297,8 +297,12 @@ collectInstructionsInBetween(Instruction &StartInst, const Instruction &EndInst,
 }
 
 bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
-                              DominatorTree &DT, const PostDominatorTree &PDT,
-                              DependenceInfo &DI) {
+                              DominatorTree &DT, const PostDominatorTree *PDT,
+                              DependenceInfo *DI) {
+  // Skip tests when we don't have PDT or DI
+  if (!PDT || !DI)
+    return false;
+
   // Cannot move itself before itself.
   if (&I == &InsertPoint)
     return false;
@@ -314,7 +318,7 @@ bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
     return reportInvalidCandidate(I, NotMovedTerminator);
 
   // TODO remove this limitation.
-  if (!isControlFlowEquivalent(I, InsertPoint, DT, PDT))
+  if (!isControlFlowEquivalent(I, InsertPoint, DT, *PDT))
     return reportInvalidCandidate(I, NotControlFlowEquivalent);
 
   if (!DT.dominates(&InsertPoint, &I))
@@ -363,7 +367,7 @@ bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
   // StartInst to \p EndInst.
   if (std::any_of(InstsToCheck.begin(), InstsToCheck.end(),
                   [&DI, &I](Instruction *CurInst) {
-                    auto DepResult = DI.depends(&I, CurInst, true);
+                    auto DepResult = DI->depends(&I, CurInst, true);
                     if (DepResult &&
                         (DepResult->isOutput() || DepResult->isFlow() ||
                          DepResult->isAnti()))
@@ -376,8 +380,8 @@ bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
 }
 
 bool llvm::isSafeToMoveBefore(BasicBlock &BB, Instruction &InsertPoint,
-                              DominatorTree &DT, const PostDominatorTree &PDT,
-                              DependenceInfo &DI) {
+                              DominatorTree &DT, const PostDominatorTree *PDT,
+                              DependenceInfo *DI) {
   return llvm::all_of(BB, [&](Instruction &I) {
     if (BB.getTerminator() == &I)
       return true;
@@ -396,7 +400,7 @@ void llvm::moveInstructionsToTheBeginning(BasicBlock &FromBB, BasicBlock &ToBB,
     // Increment the iterator before modifying FromBB.
     ++It;
 
-    if (isSafeToMoveBefore(I, *MovePos, DT, PDT, DI))
+    if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
       I.moveBefore(MovePos);
   }
 }
@@ -408,7 +412,7 @@ void llvm::moveInstructionsToTheEnd(BasicBlock &FromBB, BasicBlock &ToBB,
   Instruction *MovePos = ToBB.getTerminator();
   while (FromBB.size() > 1) {
     Instruction &I = FromBB.front();
-    if (isSafeToMoveBefore(I, *MovePos, DT, PDT, DI))
+    if (isSafeToMoveBefore(I, *MovePos, DT, &PDT, &DI))
       I.moveBefore(MovePos);
   }
 }

diff  --git a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
index 27e140d81a98..f9f1b235d0d0 100644
--- a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
@@ -505,48 +505,50 @@ TEST(CodeMoverUtils, IsSafeToMoveTest1) {
         // Can move after CI_safecall, as it does not throw, not synchronize, or
         // must return.
         EXPECT_TRUE(isSafeToMoveBefore(*CI_safecall->getPrevNode(),
-                                       *CI_safecall->getNextNode(), DT, PDT,
-                                       DI));
+                                       *CI_safecall->getNextNode(), DT, &PDT,
+                                       &DI));
 
         // Cannot move CI_unsafecall, as it may throw.
         EXPECT_FALSE(isSafeToMoveBefore(*CI_unsafecall->getNextNode(),
-                                        *CI_unsafecall, DT, PDT, DI));
+                                        *CI_unsafecall, DT, &PDT, &DI));
 
         // Moving instruction to non control flow equivalent places are not
         // supported.
-        EXPECT_FALSE(
-            isSafeToMoveBefore(*SI_A5, *Entry->getTerminator(), DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*SI_A5, *Entry->getTerminator(), DT,
+                                        &PDT, &DI));
 
         // Moving PHINode is not supported.
         EXPECT_FALSE(isSafeToMoveBefore(PN, *PN.getNextNode()->getNextNode(),
-                                        DT, PDT, DI));
+                                        DT, &PDT, &DI));
 
         // Cannot move non-PHINode before PHINode.
-        EXPECT_FALSE(isSafeToMoveBefore(*PN.getNextNode(), PN, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*PN.getNextNode(), PN, DT, &PDT, &DI));
 
         // Moving Terminator is not supported.
         EXPECT_FALSE(isSafeToMoveBefore(*Entry->getTerminator(),
-                                        *PN.getNextNode(), DT, PDT, DI));
+                                        *PN.getNextNode(), DT, &PDT, &DI));
 
         // Cannot move %arrayidx_A after SI, as SI is its user.
         EXPECT_FALSE(isSafeToMoveBefore(*SI->getPrevNode(), *SI->getNextNode(),
-                                        DT, PDT, DI));
+                                        DT, &PDT, &DI));
 
         // Cannot move SI before %arrayidx_A, as %arrayidx_A is its operand.
-        EXPECT_FALSE(isSafeToMoveBefore(*SI, *SI->getPrevNode(), DT, PDT, DI));
+        EXPECT_FALSE(
+            isSafeToMoveBefore(*SI, *SI->getPrevNode(), DT, &PDT, &DI));
 
         // Cannot move LI2 after SI_A6, as there is a flow dependence.
         EXPECT_FALSE(
-            isSafeToMoveBefore(*LI2, *SI_A6->getNextNode(), DT, PDT, DI));
+            isSafeToMoveBefore(*LI2, *SI_A6->getNextNode(), DT, &PDT, &DI));
 
         // Cannot move SI after LI1, as there is a anti dependence.
-        EXPECT_FALSE(isSafeToMoveBefore(*SI, *LI1->getNextNode(), DT, PDT, DI));
+        EXPECT_FALSE(
+            isSafeToMoveBefore(*SI, *LI1->getNextNode(), DT, &PDT, &DI));
 
         // Cannot move SI_A5 after SI, as there is a output dependence.
-        EXPECT_FALSE(isSafeToMoveBefore(*SI_A5, *LI1, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*SI_A5, *LI1, DT, &PDT, &DI));
 
         // Can move LI2 before LI1, as there is only an input dependence.
-        EXPECT_TRUE(isSafeToMoveBefore(*LI2, *LI1, DT, PDT, DI));
+        EXPECT_TRUE(isSafeToMoveBefore(*LI2, *LI1, DT, &PDT, &DI));
       });
 }
 
@@ -578,11 +580,11 @@ TEST(CodeMoverUtils, IsSafeToMoveTest2) {
         Instruction *SubInst = getInstructionByName(F, "sub");
 
         // Cannot move as %user uses %add and %sub doesn't dominates %user.
-        EXPECT_FALSE(isSafeToMoveBefore(*AddInst, *SubInst, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*AddInst, *SubInst, DT, &PDT, &DI));
 
         // Cannot move as %sub_op0 is an operand of %sub and %add doesn't
         // dominates %sub_op0.
-        EXPECT_FALSE(isSafeToMoveBefore(*SubInst, *AddInst, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*SubInst, *AddInst, DT, &PDT, &DI));
       });
 }
 
@@ -611,7 +613,7 @@ TEST(CodeMoverUtils, IsSafeToMoveTest3) {
 
         // Can move as the incoming block of %inc for %i (%for.latch) dominated
         // by %cmp.
-        EXPECT_TRUE(isSafeToMoveBefore(*IncInst, *CmpInst, DT, PDT, DI));
+        EXPECT_TRUE(isSafeToMoveBefore(*IncInst, *CmpInst, DT, &PDT, &DI));
       });
 }
 
@@ -643,10 +645,10 @@ TEST(CodeMoverUtils, IsSafeToMoveTest4) {
         Instruction *SubInst = getInstructionByName(F, "sub");
 
         // Cannot move as %user uses %add and %sub doesn't dominates %user.
-        EXPECT_FALSE(isSafeToMoveBefore(*AddInst, *SubInst, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*AddInst, *SubInst, DT, &PDT, &DI));
 
         // Cannot move as %sub_op0 is an operand of %sub and %add doesn't
         // dominates %sub_op0.
-        EXPECT_FALSE(isSafeToMoveBefore(*SubInst, *AddInst, DT, PDT, DI));
+        EXPECT_FALSE(isSafeToMoveBefore(*SubInst, *AddInst, DT, &PDT, &DI));
       });
 }


        


More information about the llvm-commits mailing list