[Loop] Move block and loop dispo invalidation to makeLoopInvariant.
authorFlorian Hahn <flo@fhahn.com>
Fri, 14 Oct 2022 20:58:14 +0000 (21:58 +0100)
committerFlorian Hahn <flo@fhahn.com>
Fri, 14 Oct 2022 20:58:14 +0000 (21:58 +0100)
makeLoopInvariant may recursively move its operands to make them
invariant, before moving the passed in instruction. Those recursively
moved instructions are currently missed when invalidating block and loop
dispositions.

To address this, move the invalidation code to Loop::makeLoopInvariant.

Fixes #58314.

Reviewed By: nikic

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

llvm/include/llvm/Analysis/LoopInfo.h
llvm/lib/Analysis/LoopInfo.cpp
llvm/lib/Transforms/Scalar/LoopDeletion.cpp
llvm/lib/Transforms/Utils/LoopSimplify.cpp
llvm/test/Transforms/LoopDeletion/invalidate-scev-after-hoisting.ll [moved from llvm/test/Transforms/LoopDeletion/pr57837-invalidate-scev-after-hoisting.ll with 82% similarity]

index 5a4f8f1..eb0b9ef 100644 (file)
@@ -583,7 +583,8 @@ public:
   ///
   bool makeLoopInvariant(Value *V, bool &Changed,
                          Instruction *InsertPt = nullptr,
-                         MemorySSAUpdater *MSSAU = nullptr) const;
+                         MemorySSAUpdater *MSSAU = nullptr,
+                         ScalarEvolution *SE = nullptr) const;
 
   /// If the given instruction is inside of the loop and it can be hoisted, do
   /// so to make it trivially loop-invariant.
@@ -597,7 +598,8 @@ public:
   ///
   bool makeLoopInvariant(Instruction *I, bool &Changed,
                          Instruction *InsertPt = nullptr,
-                         MemorySSAUpdater *MSSAU = nullptr) const;
+                         MemorySSAUpdater *MSSAU = nullptr,
+                         ScalarEvolution *SE = nullptr) const;
 
   /// Check to see if the loop has a canonical induction variable: an integer
   /// recurrence that starts at 0 and increments by one each time through the
index 693b9eb..90077aa 100644 (file)
@@ -68,15 +68,16 @@ bool Loop::hasLoopInvariantOperands(const Instruction *I) const {
 }
 
 bool Loop::makeLoopInvariant(Value *V, bool &Changed, Instruction *InsertPt,
-                             MemorySSAUpdater *MSSAU) const {
+                             MemorySSAUpdater *MSSAU,
+                             ScalarEvolution *SE) const {
   if (Instruction *I = dyn_cast<Instruction>(V))
-    return makeLoopInvariant(I, Changed, InsertPt, MSSAU);
+    return makeLoopInvariant(I, Changed, InsertPt, MSSAU, SE);
   return true; // All non-instructions are loop-invariant.
 }
 
 bool Loop::makeLoopInvariant(Instruction *I, bool &Changed,
-                             Instruction *InsertPt,
-                             MemorySSAUpdater *MSSAU) const {
+                             Instruction *InsertPt, MemorySSAUpdater *MSSAU,
+                             ScalarEvolution *SE) const {
   // Test if the value is already loop-invariant.
   if (isLoopInvariant(I))
     return true;
@@ -97,7 +98,7 @@ bool Loop::makeLoopInvariant(Instruction *I, bool &Changed,
   }
   // Don't hoist instructions with loop-variant operands.
   for (Value *Operand : I->operands())
-    if (!makeLoopInvariant(Operand, Changed, InsertPt, MSSAU))
+    if (!makeLoopInvariant(Operand, Changed, InsertPt, MSSAU, SE))
       return false;
 
   // Hoist.
@@ -113,6 +114,9 @@ bool Loop::makeLoopInvariant(Instruction *I, bool &Changed,
   // information to the optimizer.
   I->dropUnknownNonDebugMetadata();
 
+  if (SE)
+    SE->forgetBlockAndLoopDispositions(I);
+
   Changed = true;
   return true;
 }
index f19f53e..bcc038c 100644 (file)
@@ -90,17 +90,11 @@ static bool isLoopDead(Loop *L, ScalarEvolution &SE,
         break;
 
       if (Instruction *I = dyn_cast<Instruction>(incoming)) {
-        bool InstrMoved = false;
-        if (!L->makeLoopInvariant(I, InstrMoved, Preheader->getTerminator())) {
+        if (!L->makeLoopInvariant(I, Changed, Preheader->getTerminator(),
+                                  /*MSSAU=*/nullptr, &SE)) {
           AllEntriesInvariant = false;
           break;
         }
-        Changed |= InstrMoved;
-        if (InstrMoved) {
-          // Moving I to a different location may change its block disposition,
-          // so invalidate its SCEV.
-          SE.forgetBlockAndLoopDispositions(I);
-        }
       }
     }
   }
index f247a7e..8943b4b 100644 (file)
@@ -647,19 +647,12 @@ ReprocessLoop:
         Instruction *Inst = &*I++;
         if (Inst == CI)
           continue;
-        bool InstInvariant = false;
         if (!L->makeLoopInvariant(
-                Inst, InstInvariant,
-                Preheader ? Preheader->getTerminator() : nullptr, MSSAU)) {
+                Inst, AnyInvariant,
+                Preheader ? Preheader->getTerminator() : nullptr, MSSAU, SE)) {
           AllInvariant = false;
           break;
         }
-        if (InstInvariant && SE) {
-          // The loop disposition of all SCEV expressions that depend on any
-          // hoisted values have also changed.
-          SE->forgetBlockAndLoopDispositions(Inst);
-        }
-        AnyInvariant |= InstInvariant;
       }
       if (AnyInvariant)
         Changed = true;
@@ -9,8 +9,8 @@
 ; CHECK-NEXT: Loop %inner: max backedge-taken count is 405
 ; CHECK-NEXT: Loop %inner: Predicated backedge-taken count is (405 + %invar)<nuw><nsw>
 
-define void @test() {
-; CHECK-LABEL: @test(
+define void @test_pr57837() {
+; CHECK-LABEL: @test_pr57837(
 ; CHECK-NEXT:  bb:
 ; CHECK-NEXT:    br label [[OUTER_HEADER:%.*]]
 ; CHECK:       outer.header:
@@ -126,3 +126,37 @@ outer.latch:
   %outer.iv.next = add nsw i32 %l, %trunc
   br label %outer.header
 }
+
+define void @test_pr58314() {
+; CHECK-LABEL: @test_pr58314(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[OUTER_HEADER:%.*]]
+; CHECK:       outer.header:
+; CHECK-NEXT:    [[C:%.*]] = icmp ne i16 0, 0
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[C]], i1 false, i1 true
+; CHECK-NEXT:    br label [[INNER:%.*]]
+; CHECK:       inner:
+; CHECK-NEXT:    br i1 true, label [[INNER]], label [[OUTER_LATCH:%.*]]
+; CHECK:       outer.latch:
+; CHECK-NEXT:    [[SEL_LCSSA:%.*]] = phi i1 [ [[SEL]], [[INNER]] ]
+; CHECK-NEXT:    br i1 [[SEL_LCSSA]], label [[OUTER_HEADER]], label [[EXIT:%.*]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %outer.header
+
+outer.header:
+  br label %inner
+
+inner:
+  %c = icmp ne i16 0, 0
+  %sel = select i1 %c, i1 false, i1 true
+  br i1 true, label %inner, label %outer.latch
+
+outer.latch:
+  br i1 %sel, label %outer.header, label %exit
+
+exit:
+  ret void
+}