[llvm] 0e36288 - [LoopPredication] Report changes correctly when attempting loop exit predication

[Daniil Suchkov via llvm-commits]

Author: Daniil Suchkov
Date: 2021-09-16T22:49:55Z
New Revision: 0e362883182db53da9d6dd8e90f8eafb4e3b916c

URL: https://github.com/llvm/llvm-project/commit/0e362883182db53da9d6dd8e90f8eafb4e3b916c
DIFF: https://github.com/llvm/llvm-project/commit/0e362883182db53da9d6dd8e90f8eafb4e3b916c.diff

LOG: [LoopPredication] Report changes correctly when attempting loop exit predication

To make the IR easier to analyze, this pass makes some minor transformations.
After that, even if it doesn't decide to optimize anything, it can't report that
it changed nothing and preserved all the analyses.

Reviewed By: reames

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

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/LoopPredication.cpp
    llvm/test/Transforms/LoopPredication/invalidate-analyses.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/LoopPredication.cpp b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
index 0d5d7e7d01aa..b57d35a10775 100644
--- a/llvm/lib/Transforms/Scalar/LoopPredication.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
@@ -1086,28 +1086,26 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
   // widen so that we gain ability to analyze it's exit count and perform this
   // transform.  TODO: It'd be nice to know for sure the exit became
   // analyzeable after dropping widenability.
-  {
-    bool Invalidate = false;
+  bool ChangedLoop = false;
 
-    for (auto *ExitingBB : ExitingBlocks) {
-      if (LI->getLoopFor(ExitingBB) != L)
-        continue;
+  for (auto *ExitingBB : ExitingBlocks) {
+    if (LI->getLoopFor(ExitingBB) != L)
+      continue;
 
-      auto *BI = dyn_cast<BranchInst>(ExitingBB->getTerminator());
-      if (!BI)
-        continue;
+    auto *BI = dyn_cast<BranchInst>(ExitingBB->getTerminator());
+    if (!BI)
+      continue;
 
-      Use *Cond, *WC;
-      BasicBlock *IfTrueBB, *IfFalseBB;
-      if (parseWidenableBranch(BI, Cond, WC, IfTrueBB, IfFalseBB) &&
-          L->contains(IfTrueBB)) {
-        WC->set(ConstantInt::getTrue(IfTrueBB->getContext()));
-        Invalidate = true;
-      }
+    Use *Cond, *WC;
+    BasicBlock *IfTrueBB, *IfFalseBB;
+    if (parseWidenableBranch(BI, Cond, WC, IfTrueBB, IfFalseBB) &&
+        L->contains(IfTrueBB)) {
+      WC->set(ConstantInt::getTrue(IfTrueBB->getContext()));
+      ChangedLoop = true;
     }
-    if (Invalidate)
-      SE->forgetLoop(L);
   }
+  if (ChangedLoop)
+    SE->forgetLoop(L);
 
   // The use of umin(all analyzeable exits) instead of latch is subtle, but
   // important for profitability.  We may have a loop which hasn't been fully
@@ -1119,13 +1117,15 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
   if (isa<SCEVCouldNotCompute>(MinEC) || MinEC->getType()->isPointerTy() ||
       !SE->isLoopInvariant(MinEC, L) ||
       !isSafeToExpandAt(MinEC, WidenableBR, *SE))
-    return false;
+    return ChangedLoop;
 
   // Subtlety: We need to avoid inserting additional uses of the WC.  We know
   // that it can only have one transitive use at the moment, and thus moving
   // that use to just before the branch and inserting code before it and then
   // modifying the operand is legal.
   auto *IP = cast<Instruction>(WidenableBR->getCondition());
+  // Here we unconditionally modify the IR, so after this point we should return
+  // only `true`!
   IP->moveBefore(WidenableBR);
   if (MSSAU)
     if (auto *MUD = MSSAU->getMemorySSA()->getMemoryAccess(IP))
@@ -1134,7 +1134,7 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
   Rewriter.setInsertPoint(IP);
   IRBuilder<> B(IP);
 
-  bool Changed = false;
+  bool InvalidateLoop = false;
   Value *MinECV = nullptr; // lazily generated if needed
   for (BasicBlock *ExitingBB : ExitingBlocks) {
     // If our exiting block exits multiple loops, we can only rewrite the
@@ -1191,16 +1191,18 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
 
     Value *OldCond = BI->getCondition();
     BI->setCondition(ConstantInt::get(OldCond->getType(), !ExitIfTrue));
-    Changed = true;
+    InvalidateLoop = true;
   }
 
-  if (Changed)
+  if (InvalidateLoop)
     // We just mutated a bunch of loop exits changing there exit counts
     // widely.  We need to force recomputation of the exit counts given these
     // changes.  Note that all of the inserted exits are never taken, and
     // should be removed next time the CFG is modified.
     SE->forgetLoop(L);
-  return Changed;
+
+  // Always return `true` since we have moved the WidenableBR's condition.
+  return true;
 }
 
 bool LoopPredication::runOnLoop(Loop *Loop) {

diff  --git a/llvm/test/Transforms/LoopPredication/invalidate-analyses.ll b/llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
index 9c1812a9ae9d..ac5a84ce7cf2 100644
--- a/llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
+++ b/llvm/test/Transforms/LoopPredication/invalidate-analyses.ll
@@ -1,6 +1,5 @@
 ; RUN: opt -S -passes='require<scalar-evolution>,loop-mssa(loop-predication)' -debug-pass-manager < %s 2>&1 | FileCheck %s
 
-; FIXME: We should invalidate PreservedCFGCheckerAnalysis after LoopPredicationPass!
 ; NOTE: PreservedCFGCheckerAnalysis is an arbitrary analysis that just happens
 ;       to be calculated before this pass and isn't preserved by it. If after
 ;       your change this analysis is preserved by the pass, please update this
@@ -8,9 +7,10 @@
 
 ; CHECK: Running analysis: PreservedCFGCheckerAnalysis on drop_a_wc_and_leave_early
 ; CHECK: Running pass: LoopPredicationPass on Loop at depth 1 containing: %loop<header><exiting>,%guarded<exiting>,%guarded2<latch><exiting>
+; CHECK-NEXT: Invalidating analysis: PreservedCFGCheckerAnalysis on drop_a_wc_and_leave_early
 ; CHECK-NEXT: Running analysis: PreservedCFGCheckerAnalysis on drop_a_wc_and_leave
 ; CHECK: Running pass: LoopPredicationPass on Loop at depth 1 containing: %loop<header><exiting>,%guarded<exiting>,%guarded2<latch><exiting>
-; CHECK-NEXT: Running pass: VerifierPass
+; CHECK-NEXT: Invalidating analysis: PreservedCFGCheckerAnalysis on drop_a_wc_and_leave
 
 
 ; This test makes the pass drop its attempts to optimize the exit condition in