[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

diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index 6d63277..914fadc 100644 (file)
--- 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 713743d..49a6d93 100644 (file)
--- 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:  ]
+}