[llvm] db04ff4 - [SimpleLoopUnswitch] Add non-empty unreachable block check to exit cases removed.

[Alina Sbirlea via llvm-commits]

Author: Alina Sbirlea
Date: 2020-05-13T12:38:37-07:00
New Revision: db04ff4b6bc3fbf5618556c52f8dd57a00790648

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

LOG: [SimpleLoopUnswitch] Add non-empty unreachable block check to exit cases removed.

Summary:
Update check to include the check for unreachable.

Basic blocks ending in unreachable are special cased, as these blocks may be already unswitched. Before this patch this check is only done for the default destination.
The condition for the exit cases and the default case must be the same, because we should never leave edges from the switch instruction to a basic block that we are unswitching. In PR45355 we still have a remaining edge (that we're attempting to remove from the DT) because its the default edge to an unreachable-terminated block where we unswitch a case edge to that block.

Resolves PR45355.

Reviewers: chandlerc

Subscribers: hiraditya, uabelho, llvm-commits

Tags: #llvm

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

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
    llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index 6d63277e85b7..914fadc40d74 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -598,6 +598,11 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
 
   auto *ParentBB = SI.getParent();
 
+  // The same check must be used both for the default and the exit cases. We
+  // should never leave edges from the switch instruction to a basic block that
+  // we are unswitching, hence the condition used to determine the default case
+  // needs to also be used to populate ExitCaseIndices, which is then used to
+  // remove cases from the switch.
   auto IsTriviallyUnswitchableExitBlock = [&](BasicBlock &BBToCheck) {
     // BBToCheck is not an exit block if it is inside loop L.
     if (L.contains(&BBToCheck))
@@ -616,12 +621,9 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
   };
 
   SmallVector<int, 4> ExitCaseIndices;
-  for (auto Case : SI.cases()) {
-    auto *SuccBB = Case.getCaseSuccessor();
-    if (!L.contains(SuccBB) &&
-        areLoopExitPHIsLoopInvariant(L, *ParentBB, *SuccBB))
+  for (auto Case : SI.cases())
+    if (IsTriviallyUnswitchableExitBlock(*Case.getCaseSuccessor()))
       ExitCaseIndices.push_back(Case.getCaseIndex());
-  }
   BasicBlock *DefaultExitBB = nullptr;
   SwitchInstProfUpdateWrapper::CaseWeightOpt DefaultCaseWeight =
       SwitchInstProfUpdateWrapper::getSuccessorWeight(SI, 0);

diff  --git a/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll b/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll
index 713743db6500..49a6d93d0bef 100644
--- a/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch.ll
@@ -1274,14 +1274,67 @@ NonEmptyUnreachableBlock:
 ; CHECK-NEXT:  %cleanup.dest.slot.0 = select i1 undef, i32 5, i32 undef
 ; CHECK-NEXT:  switch i32 %cleanup.dest.slot.0, label %NonEmptyUnreachableBlock [
 ; CHECK-NEXT:    i32 1, label %NonEmptyUnreachableBlock
-; CHECK-NEXT:    i32 2, label %loop.loopexit
 ; CHECK-NEXT:    i32 0, label %loop.split
+; CHECK-NEXT:    i32 2, label %loop.split
+; CHECK-NEXT:  ]
+
+; CHECK:loop.split:
+; CHECK-NEXT:  br label %for.cond
+
+; CHECK:for.cond:
+; CHECK-NEXT:  switch i32 %cleanup.dest.slot.0, label %loop.loopexit [
+; CHECK-NEXT:    i32 0, label %for.cond
+; CHECK-NEXT:  ]
+
+; CHECK:loop.loopexit:
+; CHECK-NEXT:  unreachable
+
+; CHECK:NonEmptyUnreachableBlock:
+; CHECK-NEXT:  call void @f()
+; CHECK-NEXT:  call void @g()
+; CHECK-NEXT:  unreachable
+}
+
+define void @test_unswitch_switch_with_nonempty_unreachable2() {
+; CHECK-LABEL: @test_unswitch_switch_with_nonempty_unreachable2()
+entry:
+  br label %loop
+
+loop:
+  %cleanup.dest.slot.0 = select i1 undef, i32 5, i32 undef
+  br label %for.cond
+
+for.cond:
+  switch i32 %cleanup.dest.slot.0, label %for.cond [
+    i32 0, label %for.cond
+    i32 1, label %NonEmptyUnreachableBlock
+    i32 2, label %loop.loopexit
+  ]
+
+loop.loopexit:
+  unreachable
+
+NonEmptyUnreachableBlock:
+  call void @f()
+  call void @g()
+  unreachable
+
+; CHECK:loop:
+; CHECK-NEXT:  %cleanup.dest.slot.0 = select i1 undef, i32 5, i32 undef
+; CHECK-NEXT:  switch i32 %cleanup.dest.slot.0, label %loop.split [
+; CHECK-NEXT:    i32 1, label %NonEmptyUnreachableBlock
 ; CHECK-NEXT:  ]
 
 ; CHECK:loop.split:
 ; CHECK-NEXT:  br label %for.cond
 
 ; CHECK:for.cond:
+; CHECK-NEXT:  switch i32 %cleanup.dest.slot.0, label %for.cond.backedge [
+; CHECK-NEXT:    i32 0, label %for.cond.backedge
+; CHECK-NEXT:    i32 2, label %loop.loopexit
+; CHECK-NEXT:  ]
+
+; CHECK:for.cond.backedge:
 ; CHECK-NEXT:  br label %for.cond
 
 ; CHECK:loop.loopexit:
@@ -1292,3 +1345,34 @@ NonEmptyUnreachableBlock:
 ; CHECK-NEXT:  call void @g()
 ; CHECK-NEXT:  unreachable
 }
+
+; PR45355
+define void @test_unswitch_switch_with_duplicate_edge() {
+; CHECK-LABEL: @test_unswitch_switch_with_duplicate_edge()
+entry:
+  br label %lbl1
+
+lbl1:                                             ; preds = %entry
+  %cleanup.dest.slot.0 = select i1 undef, i32 5, i32 undef
+  br label %for.cond1
+
+for.cond1:                                        ; preds = %for.cond1, %lbl1
+  switch i32 %cleanup.dest.slot.0, label %UnifiedUnreachableBlock [
+    i32 0, label %for.cond1
+    i32 5, label %UnifiedUnreachableBlock
+    i32 2, label %lbl1.loopexit
+  ]
+
+UnifiedUnreachableBlock:                          ; preds = %for.cond1, %for.cond1
+  unreachable
+
+lbl1.loopexit:                                    ; preds = %for.cond1
+  unreachable
+
+; CHECK: for.cond1:
+; CHECK-NEXT:  switch i32 %cleanup.dest.slot.0, label %UnifiedUnreachableBlock [
+; CHECK-NEXT:    i32 0, label %for.cond1
+; CHECK-NEXT:    i32 5, label %UnifiedUnreachableBlock
+; CHECK-NEXT:    i32 2, label %lbl1.loopexit
+; CHECK-NEXT:  ]
+}