[llvm] 4a699ae - [LoopSimplifyCFG] Check predecessors of exits before marking them dead.

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2022-03-18T08:54:44Z
New Revision: 4a699ae9c6a6fde93ec4714d79025df58eb62a72

URL: https://github.com/llvm/llvm-project/commit/4a699ae9c6a6fde93ec4714d79025df58eb62a72
DIFF: https://github.com/llvm/llvm-project/commit/4a699ae9c6a6fde93ec4714d79025df58eb62a72.diff

LOG: [LoopSimplifyCFG] Check predecessors of exits before marking them dead.

LoopSimplifyCFG may process loops that are not in
loop-simplify/canonical form. For loops not in canonical form, exit
blocks may be reachable from non-loop blocks and we cannot consider them
as dead if they only are not reachable from the loop itself.

Unfortunately the smallest test I could come up with requires running
multiple passes:
    -passes='loop-mssa(loop-instsimplify,loop-simplifycfg,simple-loop-unswitch)'

The reason is that loops are canonicalized at the beginning of loop
pipelines, so a later transform has to break canonical form in a way
that breaks LoopSimplifyCFG's dead-exit analysis.

Alternatively we could try to require all loop passes to maintain
canonical form. That in turn would also require additional verification.

Fixes #54023, #49931.

Reviewed By: nikic

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

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
    llvm/test/Transforms/LoopSimplifyCFG/loop-not-in-simplify-form.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp b/llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
index 0f85d6cccb05d..81368bca9bbff 100644
--- a/llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
@@ -254,13 +254,17 @@ class ConstantTerminatorFoldingImpl {
     assert(L.getNumBlocks() == LiveLoopBlocks.size() + DeadLoopBlocks.size() &&
            "Malformed block sets?");
 
-    // Now, all exit blocks that are not marked as live are dead.
+    // Now, all exit blocks that are not marked as live are dead, if all their
+    // predecessors are in the loop. This may not be the case, as the input loop
+    // may not by in loop-simplify/canonical form.
     SmallVector<BasicBlock *, 8> ExitBlocks;
     L.getExitBlocks(ExitBlocks);
     SmallPtrSet<BasicBlock *, 8> UniqueDeadExits;
     for (auto *ExitBlock : ExitBlocks)
       if (!LiveExitBlocks.count(ExitBlock) &&
-          UniqueDeadExits.insert(ExitBlock).second)
+          UniqueDeadExits.insert(ExitBlock).second &&
+          all_of(predecessors(ExitBlock),
+                 [this](BasicBlock *Pred) { return L.contains(Pred); }))
         DeadExitBlocks.push_back(ExitBlock);
 
     // Whether or not the edge From->To will still be present in graph after the

diff  --git a/llvm/test/Transforms/LoopSimplifyCFG/loop-not-in-simplify-form.ll b/llvm/test/Transforms/LoopSimplifyCFG/loop-not-in-simplify-form.ll
index 88cdc8c5ddab7..2cd7c695ebcd0 100644
--- a/llvm/test/Transforms/LoopSimplifyCFG/loop-not-in-simplify-form.ll
+++ b/llvm/test/Transforms/LoopSimplifyCFG/loop-not-in-simplify-form.ll
@@ -4,14 +4,9 @@
 ; Test case from PR54023. After SimpleLoopUnswitch, one of the loops processed
 ; by LoopSimplifyCFG will have exit blocks with predecessors outside the loop
 ; (i.e. it is not in loop-simplify/canonical form).
-; FIXME: currently %res gets incorrectly replaced with undef.
 define i32 @test(i32 %v, i1 %c.1, i1 %c.2) {
 ; CHECK-LABEL: @test(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    switch i32 0, label [[ENTRY_SPLIT:%.*]] [
-; CHECK-NEXT:    i32 1, label [[EXIT_SPLIT:%.*]]
-; CHECK-NEXT:    ]
-; CHECK:       entry.split:
 ; CHECK-NEXT:    br label [[OUTER_HEADER:%.*]]
 ; CHECK:       outer.header:
 ; CHECK-NEXT:    br i1 [[C_1:%.*]], label [[OUTER_LATCH:%.*]], label [[INNER_HEADER_PREHEADER:%.*]]
@@ -28,9 +23,10 @@ define i32 @test(i32 %v, i1 %c.1, i1 %c.2) {
 ; CHECK-NEXT:    br label [[OUTER_HEADER]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    [[RES:%.*]] = phi i32 [ [[V]], [[INNER_HEADER]] ]
-; CHECK-NEXT:    br label [[EXIT_SPLIT]]
+; CHECK-NEXT:    br label [[EXIT_SPLIT:%.*]]
 ; CHECK:       exit.split:
-; CHECK-NEXT:    ret i32 undef
+; CHECK-NEXT:    [[RES_SPLIT:%.*]] = phi i32 [ [[RES]], [[EXIT]] ]
+; CHECK-NEXT:    ret i32 [[RES_SPLIT]]
 ;
 entry:
   br label %outer.header