[SimplifyCFG] Hoist common code when succ is unreachable block #165570

Camsyn · 2025-10-29T14:03:17Z

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 dtcxzyw/llvm-opt-benchmark#2989 for details.

Alive2 proof: https://alive2.llvm.org/ce/z/OJOw0s
Promising optimization impact: dtcxzyw/llvm-opt-benchmark#2995

llvmbot · 2025-10-29T14:03:53Z

@llvm/pr-subscribers-llvm-transforms

Author: Kunqiu Chen (Camsyn)

Changes

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 dtcxzyw/llvm-opt-benchmark#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 --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
+}

Camsyn · 2025-10-29T14:04:17Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+    if (Succ->hasAddressTaken())
      return false;
-
+    if (Succ->getSinglePredecessor())


Moreover, should us relax this condition to support mutl-cases destination in switch?

Suggested change

if (Succ->getSinglePredecessor())

if (Succ->getUniqPredecessor())

This makes sense to me -- let's do it as a followup.

nikic · 2025-10-29T15:19:47Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+    // 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()))


This is not quite the right condition, because it does not exclude other instructions before the unreachable (e.g. `call llvm.trap + unreachable). You need the first instruction to be unreachable.

nikic · 2025-10-29T15:36:02Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+    // 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->getFirstNonPHIOrDbgOrLifetime()))


You can just use begin() here. There is no need to skip phi nodes or lifetimes in this context.

I'm just scared of such a weird block (in a crazy way) 😂

However, this should not be the case with standard blocks

nikic

LGTM

nikic · 2025-10-29T16:00:49Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

-
+    if (Succ->getSinglePredecessor())
+      continue;
+    // If Succ has >1 predecessors, continue to check if the Succ is terminated


terminated by an unreachable inst -> contains only an unreachable inst

…165570) 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 dtcxzyw/llvm-opt-benchmark#2989 for details. Alive2 proof: https://alive2.llvm.org/ce/z/OJOw0s Promising optimization impact: dtcxzyw/llvm-opt-benchmark#2995

Camsyn added 3 commits October 29, 2025 21:43

Add a test before commit

29e07b8

[SimplifyCFG] Hoist common code when succ is unreachable block

cc6ca09

Regenerate the test after commit

779580a

Camsyn requested review from dianqk, dtcxzyw and nikic October 29, 2025 14:03

Camsyn self-assigned this Oct 29, 2025

llvmbot added the llvm:transforms label Oct 29, 2025

Camsyn commented Oct 29, 2025

View reviewed changes

Camsyn mentioned this pull request Oct 29, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

zyw-bot mentioned this pull request Oct 29, 2025

pre-commit: PR165570 dtcxzyw/llvm-opt-benchmark#2995

Closed

nikic reviewed Oct 29, 2025

View reviewed changes

Check the first inst rather than the last one

4d51e3a

nikic reviewed Oct 29, 2025

View reviewed changes

Address review comment

be325aa

nikic approved these changes Oct 29, 2025

View reviewed changes

Update comments

ed38ae3

Camsyn force-pushed the enhance-simplifycfg-hoist-common-inst branch from da55873 to ed38ae3 Compare October 29, 2025 16:07

Camsyn merged commit b2fe5d1 into llvm:main Oct 29, 2025
10 checks passed

Camsyn mentioned this pull request Oct 30, 2025

[SimplifyCFG] Hoist common code for switch multi-case destinations #165700

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SimplifyCFG] Hoist common code when succ is unreachable block #165570

[SimplifyCFG] Hoist common code when succ is unreachable block #165570

Uh oh!

Camsyn commented Oct 29, 2025 •

edited

Loading

Uh oh!

llvmbot commented Oct 29, 2025

Uh oh!

Camsyn Oct 29, 2025

Uh oh!

nikic Oct 29, 2025

Uh oh!

nikic Oct 29, 2025

Uh oh!

nikic Oct 29, 2025

Uh oh!

Camsyn Oct 29, 2025

Uh oh!

nikic left a comment

Uh oh!

nikic Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (Succ->getSinglePredecessor())
	if (Succ->getUniqPredecessor())

[SimplifyCFG] Hoist common code when succ is unreachable block #165570

[SimplifyCFG] Hoist common code when succ is unreachable block #165570

Uh oh!

Conversation

Camsyn commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 29, 2025

Uh oh!

Camsyn Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Camsyn Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Camsyn commented Oct 29, 2025 •

edited

Loading