[llvm] [SimplifyCFG] Hoist common code when succ is unreachable block (PR #165570)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Kunqiu Chen (Camsyn)

<details>
<summary>Changes</summary>

Previously, `hoistCommonCodeFromSuccessors` returned early if one of the succ of BB has >1 predecessors.

However, if the succ is an unreachable BB, we can relax the condition to perform `hoistCommonCodeFromSuccessors` based on the assumption of not reaching UB.

See discussion https://github.com/dtcxzyw/llvm-opt-benchmark/pull/2989 for details.

Alive2 proof: https://alive2.llvm.org/ce/z/OJOw0s

---
Full diff: https://github.com/llvm/llvm-project/pull/165570.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+12-3) 
- (modified) llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll (+116) 


``````````diff
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index d831c2737e5f8..7244ffbe344e6 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1866,10 +1866,19 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(Instruction *TI,
   // If either of the blocks has it's address taken, then we can't do this fold,
   // because the code we'd hoist would no longer run when we jump into the block
   // by it's address.
-  for (auto *Succ : successors(BB))
-    if (Succ->hasAddressTaken() || !Succ->getSinglePredecessor())
+  for (auto *Succ : successors(BB)) {
+    if (Succ->hasAddressTaken())
       return false;
-
+    if (Succ->getSinglePredecessor())
+      continue;
+    // If Succ has >1 predecessors, continue to check if the Succ is terminated
+    // by an `unreachable` inst. Since executing `unreachable` inst is an UB, we
+    // can relax the condition based on the assumptiom that the program would
+    // never enter Succ and trigger an UB.
+    if (isa<UnreachableInst>(Succ->getTerminator()))
+      continue;
+    return false;
+  }
   // The second of pair is a SkipFlags bitmask.
   using SuccIterPair = std::pair<BasicBlock::iterator, unsigned>;
   SmallVector<SuccIterPair, 8> SuccIterPairs;
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll b/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll
index 8ce94d1cf5b4e..98c0599ab209c 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll
@@ -486,3 +486,119 @@ else:
   call void @bar()
   ret float %op2
 }
+
+define void @test_switch_with_unreachable_block_as_default(i1 %c, i32 %x, ptr %ptr) {
+; CHECK-LABEL: @test_switch_with_unreachable_block_as_default(
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[SW1:%.*]], label [[SW2:%.*]]
+; CHECK:       sw1:
+; CHECK-NEXT:    switch i32 [[X:%.*]], label [[UNREACHABLE:%.*]] [
+; CHECK-NEXT:      i32 1, label [[COMMON_RET:%.*]]
+; CHECK-NEXT:      i32 2, label [[BAR:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       sw2:
+; CHECK-NEXT:    store i64 42, ptr [[PTR:%.*]], align 4
+; CHECK-NEXT:    br label [[COMMON_RET]]
+; CHECK:       common.ret:
+; CHECK-NEXT:    ret void
+; CHECK:       unreachable:
+; CHECK-NEXT:    unreachable
+; CHECK:       bar:
+; CHECK-NEXT:    call void @bar()
+; CHECK-NEXT:    br label [[COMMON_RET]]
+;
+  br i1 %c, label %sw1, label %sw2
+
+sw1:
+  ; This switch only exists to have an %unreachable block with multiple predecessors.
+  switch i32 %x, label %unreachable [
+  i32 1, label %foo
+  i32 2, label %bar
+  ]
+
+sw2:
+  switch i32 %x, label %unreachable [
+  i32 1, label %bb1
+  i32 2, label %bb2
+  i32 3, label %bb3
+  ]
+
+bb1:
+  store i64 42, ptr %ptr
+  ret void
+
+bb2:
+  store i64 42, ptr %ptr
+  ret void
+
+bb3:
+  store i64 42, ptr %ptr
+  ret void
+
+unreachable:
+  unreachable
+
+foo:
+  ret void
+
+bar:
+  call void @bar()
+  ret void
+}
+
+define void @test_switch_with_unreachable_block_as_case(i1 %c, i32 %x, ptr %ptr) {
+; CHECK-LABEL: @test_switch_with_unreachable_block_as_case(
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[SW1:%.*]], label [[SW2:%.*]]
+; CHECK:       sw1:
+; CHECK-NEXT:    switch i32 [[X:%.*]], label [[UNREACHABLE:%.*]] [
+; CHECK-NEXT:      i32 1, label [[COMMON_RET:%.*]]
+; CHECK-NEXT:      i32 2, label [[BAR:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       sw2:
+; CHECK-NEXT:    store i64 42, ptr [[PTR:%.*]], align 4
+; CHECK-NEXT:    br label [[COMMON_RET]]
+; CHECK:       common.ret:
+; CHECK-NEXT:    ret void
+; CHECK:       unreachable:
+; CHECK-NEXT:    unreachable
+; CHECK:       bar:
+; CHECK-NEXT:    call void @bar()
+; CHECK-NEXT:    br label [[COMMON_RET]]
+;
+  br i1 %c, label %sw1, label %sw2
+
+sw1:
+  ; This switch only exists to have an %unreachable block with multiple predecessors.
+  switch i32 %x, label %unreachable [
+  i32 1, label %foo
+  i32 2, label %bar
+  ]
+
+sw2:
+  switch i32 %x, label %bb3 [
+  i32 1, label %bb1
+  i32 2, label %bb2
+  i32 3, label %unreachable
+  ]
+
+bb1:
+  store i64 42, ptr %ptr
+  ret void
+
+bb2:
+  store i64 42, ptr %ptr
+  ret void
+
+bb3:
+  store i64 42, ptr %ptr
+  ret void
+
+unreachable:
+  unreachable
+
+foo:
+  ret void
+
+bar:
+  call void @bar()
+  ret void
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/165570