-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BranchFolding] Use isSuccessor to confirm fall through #77923
Conversation
When merging blocks, if the previous block has no any branch instruction and has one successor, the successor may be SEH landing pad and the block will always raise exception and nerver fall through to next block. We can not merge them in such case. isSuccessor should be used to confirm it can fall through to next block.
@llvm/pr-subscribers-backend-x86 Author: Haohai Wen (HaohaiWen) ChangesWhen merging blocks, if the previous block has no any branch instruction Full diff: https://github.com/llvm/llvm-project/pull/77923.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 599b7c72b2f5c6..a9f78358e57b92 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -1411,7 +1411,7 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
// This has to check PrevBB->succ_size() because EH edges are ignored by
// analyzeBranch.
if (PriorCond.empty() && !PriorTBB && MBB->pred_size() == 1 &&
- PrevBB.succ_size() == 1 &&
+ PrevBB.succ_size() == 1 && PrevBB.isSuccessor(MBB) &&
!MBB->hasAddressTaken() && !MBB->isEHPad()) {
LLVM_DEBUG(dbgs() << "\nMerging into block: " << PrevBB
<< "From MBB: " << *MBB);
diff --git a/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir b/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir
index 8eef5450e252a6..98dadbfcc17bda 100644
--- a/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir
+++ b/llvm/test/CodeGen/X86/branchfolding-landingpad-cfg.mir
@@ -49,3 +49,44 @@ body: |
bb.6:
RET 0
...
+---
+name: foo
+body: |
+ ; CHECK-LABEL: name: foo
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x7ffff800), %bb.2(0x00000800)
+ ; CHECK-NEXT: liveins: $rcx
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: renamable $eax = MOV32rm renamable $rcx, 1, $noreg, 0, $noreg
+ ; CHECK-NEXT: TEST32rr renamable $eax, renamable $eax, implicit-def $eflags
+ ; CHECK-NEXT: JCC_1 %bb.2, 14, implicit killed $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: INT 3
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: RET 0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3 (machine-block-address-taken, landing-pad, ehfunclet-entry):
+ ; CHECK-NEXT: CLEANUPRET
+ bb.0:
+ successors: %bb.1(0x7ffff800), %bb.2(0x00000800)
+ liveins: $rcx
+
+ renamable $eax = MOV32rm renamable $rcx, 1, $noreg, 0, $noreg
+ TEST32rr renamable $eax, renamable $eax, implicit-def $eflags
+ JCC_1 %bb.2, 14, implicit killed $eflags
+ JMP_1 %bb.1
+
+ bb.1:
+ successors: %bb.3(0x80000000)
+ INT 3
+
+ bb.2:
+ RET 0
+
+ bb.3 (machine-block-address-taken, landing-pad, ehfunclet-entry):
+ CLEANUPRET
+...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
When merging blocks, if the previous block has no any branch instruction
and has one successor, the successor may be SEH landing pad and the
block will always raise exception and nerver fall through to next block.
We can not merge them in such case. isSuccessor should be used to
confirm it can fall through to next block.