[BranchFolding] Fix missing predecessors of landing-pad #77608
Conversation
This test tracks BranchFolding pass which removes fall through jump and leaves landing-pad to be machine basic block of no predecessors. It would raise bug as introduced in llvm#77441.
When removing an empty machine basic block, all of its successors should be inherited by its fall through MBB. This keeps CFG as only have one entry which is required by LiveDebugValues.
|
@llvm/pr-subscribers-backend-x86 Author: None (HaohaiWen) ChangesWhen removing an empty machine basic block, all of its successors should Full diff: https://github.com/llvm/llvm-project/pull/77608.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 0801296cab49f8..599b7c72b2f5c6 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -1363,6 +1363,14 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
MachineBasicBlock *Pred = *(MBB->pred_end()-1);
Pred->ReplaceUsesOfBlockWith(MBB, &*FallThrough);
}
+ // Add rest successors of MBB to successors of FallThrough. Those
+ // successors are not directly reachable via MBB, so it should be
+ // landing-pad.
+ for (auto SI = MBB->succ_begin(), SE = MBB->succ_end(); SI != SE; ++SI)
+ if (*SI != &*FallThrough && !FallThrough->isSuccessor(*SI)) {
+ assert((*SI)->isEHPad() && "Bad CFG");
+ FallThrough->copySuccessor(MBB, SI);
+ }
// If MBB was the target of a jump table, update jump tables to go to the
// fallthrough instead.
if (MachineJumpTableInfo *MJTI = MF.getJumpTableInfo())
@@ -1624,6 +1632,15 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
} else {
DidChange = true;
PMBB->ReplaceUsesOfBlockWith(MBB, CurTBB);
+ // Add rest successors of MBB to successors of CurTBB. Those
+ // successors are not directly reachable via MBB, so it should be
+ // landing-pad.
+ for (auto SI = MBB->succ_begin(), SE = MBB->succ_end(); SI != SE;
+ ++SI)
+ if (*SI != CurTBB && !CurTBB->isSuccessor(*SI)) {
+ assert((*SI)->isEHPad() && "Bad CFG");
+ CurTBB->copySuccessor(MBB, SI);
+ }
// If this change resulted in PMBB ending in a conditional
// branch where both conditions go to the same destination,
// change this to an unconditional branch.
diff --git a/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir b/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir
new file mode 100644
index 00000000000000..8eef5450e252a6
--- /dev/null
+++ b/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir
@@ -0,0 +1,51 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
+# RUN: llc -mtriple=x86_64-pc-windows-msvc -run-pass=branch-folder -o - %s | FileCheck %s
+---
+name: main
+body: |
+ ; CHECK-LABEL: name: main
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x7ffff800), %bb.3(0x00000800)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.2(0x00000800)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: RET 0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2 (machine-block-address-taken, landing-pad, ehfunclet-entry):
+ ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: CLEANUPRET
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3 (landing-pad, ehfunclet-entry):
+ ; CHECK-NEXT: CLEANUPRET
+ bb.0:
+ successors: %bb.1(0x7ffff800), %bb.5(0x00000800)
+ JMP_1 %bb.1
+
+ bb.1:
+ successors: %bb.2(0x7ffff800), %bb.4(0x00000800)
+
+ JMP_1 %bb.2
+
+ bb.2:
+ successors: %bb.3(0x7ffff800), %bb.4(0x00000800)
+
+ JMP_1 %bb.3
+
+ bb.3:
+ successors: %bb.6(0x7ffff800)
+
+ JMP_1 %bb.6
+
+ bb.4 (machine-block-address-taken, landing-pad, ehfunclet-entry):
+ successors: %bb.5(0x80000000)
+ CLEANUPRET
+
+ bb.5 (landing-pad, ehfunclet-entry):
+ CLEANUPRET
+
+ bb.6:
+ RET 0
+...
|
If we just want to make LiveDebugValues happy, should it better to remove these died BB instead? |
This died BB was already removed by BranchFolder. See rebased test diff. We just want to salvage its successors info. |
…7441) LiveDebugValues requires CFG only has one entry. BranchFolding and MachineBlockPlacement may remove all predecessors of landing pad which leaves it to be another entry.
I see. I thought the successors are removed too. |
|
@HaohaiWen , https://lab.llvm.org/buildbot/#/builders/234
|
@vvereschaka I saw it. I've fixed this issue. |
ok, got it. I'll do it for those builders.
if you see some other failed builders because of this problem you can find their owners on the |
|
also, you can try to start a |
Thanks! I saw it has been forced to do clean up build. |
When removing an empty machine basic block, all of its successors should be inherited by its fall through MBB. This keeps CFG as only have one entry which is required by LiveDebugValues. Reland llvm#77441 as LiveDebugValues test.
When removing an empty machine basic block, all of its successors should
be inherited by its fall through MBB. This keeps CFG as only have one
entry which is required by LiveDebugValues.
Reland #77441 as LiveDebugValues test.