SCEV should forget all loops containing a deleted block.

[asbirlea]

Summary

LoopSimplifyCFG should update ScEv for all loops after a block is deleted.
If the deleted block "Succ" is part of L, then it is part of all parent loops, so forget all parent loops as well.

[greened]

Is there a test for this?

[asbirlea]

Updated to invalidate topmost loop.
Added test, courtesy of Mikael Holmén.

[mkazantsev]

LGTM

test/Transforms/LoopIdiom/scev-invalidation_topmostloop.ll

6

Please add | FileCheck %s to command line and smth like CHECK: @f1 in the test itself.

[uabelho]

Thanks for fixing this asbirlea!

[greened]

test/Transforms/LoopIdiom/scev-invalidation_topmostloop.ll

6

Is this actually testing the functionality of this change? I'm wondering how this problem was discovered. If it was a miscompilation, we should have a test that ensures we don't reintroduce the problem. If it was discovered by inspection,. then I'm guessing constructing a proper test would be difficult. I asked about a test not as a demand to produce one but rather to try to see if we have an actual observed problem we could capture in a test. If we don't I'm not sure constructing a test that doesn't actually test anything is useful.

[asbirlea]

test/Transforms/LoopIdiom/scev-invalidation_topmostloop.ll

6

The test triggers an assert without this fix, so it fails without needing any kind of CHECK. The issue was discovered by Mikael who also provided the failing test.
The added FileCheck that Maxim suggested is just a basic sanity, not really needed but it won't hurt either. Adding shortly and checking in.

[asbirlea]

Address comment.

lib/Transforms/Scalar/LoopSimplifyCFG.cpp

for (auto &Block : Blocks) {
 
     BasicBlock *Pred = Succ->getSinglePredecessor();
     if (!Pred || !Pred->getSingleSuccessor() || LI.getLoopFor(Pred) != &L)
       continue;
 
     // Merge Succ into Pred and delete it.
     MergeBlockIntoPredecessor(Succ, &DTU, &LI);
 
-    SE.forgetLoop(&L);
+    SE.forgetTopmostLoop(&L);
+
     Changed = true;
   }
 
   return Changed;
 }
 
 PreservedAnalyses LoopSimplifyCFGPass::run(Loop &L, LoopAnalysisManager &AM,
                                            LoopStandardAnalysisResults &AR,

test/Transforms/LoopIdiom/scev-invalidation_topmostloop.ll

+; RUN: opt -S -indvars -loop-idiom -verify -loop-simplifycfg -loop-idiom < %s | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: @f1()
+; CHECK-NEXT: entry:
+define void @f1() {
+entry:
+  br label %lbl1
+
+lbl1:                                             ; preds = %if.end, %entry
+  br label %for
+
+for:                                              ; preds = %if.end, %lbl1
+  br label %lor.end
+
+lor.end:                                          ; preds = %for
+  br i1 undef, label %for.end, label %if.end
+
+if.end:                                           ; preds = %lor.end
+  br i1 undef, label %lbl1, label %for
+
+for.end:                                          ; preds = %lor.end
+  ret void
+}