[BranchFolding] Avoid moving blocks to fall through to an indirect target #152916

XChy · 2025-08-10T13:43:41Z

Depend on #152591 to fix #149023.
Similar to an EH pad, there is no real advantage in "falling through" to an indirect target of an INLINEASM_BR. And multiple indirect targets of inline asm at the end of a function may be rotated infinitely.
Therefore, this patch avoids such optimization on indirect target of inline asm as fall through.

llvmbot · 2025-08-10T13:44:13Z

@llvm/pr-subscribers-backend-x86

Author: XChy (XChy)

Changes

Depend on #152591 to fix #149023.
Similar to an EH pad, there is no real advantage in "falling through" to an indirect target of an INLINEASM_BR. And multiple indirect targets of inline asm at the end of a function may be rotated infinitely.
Therefore, this patch avoids such optimization on indirect target of inline asm as fall through.

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

3 Files Affected:

(modified) llvm/lib/CodeGen/BranchFolding.cpp (+8-2)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+9-4)
(added) llvm/test/CodeGen/X86/callbr-asm-loop.ll (+41)

diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index dcfd9aad70fc5..d49c4b4a5755e 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -1776,8 +1776,9 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
       // Okay, there is no really great place to put this block.  If, however,
       // the block before this one would be a fall-through if this block were
       // removed, move this block to the end of the function. There is no real
-      // advantage in "falling through" to an EH block, so we don't want to
-      // perform this transformation for that case.
+      // advantage in "falling through" to an EH block or an indirect target of
+      // an INLINEASM_BR, so we don't want to perform this transformation for
+      // that case.
       //
       // Also, Windows EH introduced the possibility of an arbitrary number of
       // successors to a given block.  The analyzeBranch call does not consider
@@ -1787,10 +1788,15 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
       // below were performed for EH "FallThrough" blocks.  Therefore, even if
       // that appears not to be happening anymore, we should assume that it is
       // possible and not remove the "!FallThrough()->isEHPad" condition below.
+      //
+      // And inline asm branches also introduced the possibility of infinite
+      // rotation, as there are an arbitrary number of successors to a given
+      // block.
       MachineBasicBlock *PrevTBB = nullptr, *PrevFBB = nullptr;
       SmallVector<MachineOperand, 4> PrevCond;
       if (FallThrough != MF.end() &&
           !FallThrough->isEHPad() &&
+          !FallThrough->isInlineAsmBrIndirectTarget() &&
           !TII->analyzeBranch(PrevBB, PrevTBB, PrevFBB, PrevCond, true) &&
           PrevBB.isSuccessor(&*FallThrough)) {
         MBB->moveAfter(&MF.back());
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index d0815e9f51822..f096148f865cb 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12737,17 +12737,22 @@ static Register FollowCopyChain(MachineRegisterInfo &MRI, Register Reg) {
   assert(MI->getOpcode() == TargetOpcode::COPY &&
          "start of copy chain MUST be COPY");
   Reg = MI->getOperand(1).getReg();
+
+  // If the copied register in the first copy must be virtual.
+  assert(Reg.isVirtual() && "expected COPY of virtual register");
   MI = MRI.def_begin(Reg)->getParent();
+
   // There may be an optional second copy.
   if (MI->getOpcode() == TargetOpcode::COPY) {
     assert(Reg.isVirtual() && "expected COPY of virtual register");
     Reg = MI->getOperand(1).getReg();
     assert(Reg.isPhysical() && "expected COPY of physical register");
-    MI = MRI.def_begin(Reg)->getParent();
+  } else {
+    // The start of the chain must be an INLINEASM_BR.
+    assert(MI->getOpcode() == TargetOpcode::INLINEASM_BR &&
+           "end of copy chain MUST be INLINEASM_BR");
   }
-  // The start of the chain must be an INLINEASM_BR.
-  assert(MI->getOpcode() == TargetOpcode::INLINEASM_BR &&
-         "end of copy chain MUST be INLINEASM_BR");
+
   return Reg;
 }
 
diff --git a/llvm/test/CodeGen/X86/callbr-asm-loop.ll b/llvm/test/CodeGen/X86/callbr-asm-loop.ll
new file mode 100644
index 0000000000000..9e890a861e3e4
--- /dev/null
+++ b/llvm/test/CodeGen/X86/callbr-asm-loop.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+
+; RUN: llc -O1 -mtriple=i686-- < %s | FileCheck %s
+
+; Test that causes multiple defs of %eax.
+; FIXME: The testcase hangs with -O1/2/3 enabled.
+define i32 @loop1() {
+; CHECK-LABEL: loop1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    .p2align 4
+; CHECK-NEXT:  .LBB0_1: # %tailrecurse
+; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    movl $1, %edx
+; CHECK-NEXT:    #APP
+; CHECK-NEXT:    #NO_APP
+; CHECK-NEXT:    jmp .LBB0_1
+; CHECK-NEXT:  .LBB0_2: # Inline asm indirect target
+; CHECK-NEXT:    # %tailrecurse.tailrecurse_crit_edge
+; CHECK-NEXT:    # in Loop: Header=BB0_1 Depth=1
+; CHECK-NEXT:    # Label of block must be emitted
+; CHECK-NEXT:    jmp .LBB0_1
+; CHECK-NEXT:  .LBB0_3: # Inline asm indirect target
+; CHECK-NEXT:    # %lab2.split
+; CHECK-NEXT:    # Label of block must be emitted
+; CHECK-NEXT:    movl %edx, %eax
+; CHECK-NEXT:    retl
+entry:
+  br label %tailrecurse
+
+tailrecurse:
+  %0 = callbr { i32, i32 } asm "", "={ax},={dx},0,1,!i,!i"(i32 0, i32 1) #1
+          to label %tailrecurse.backedge [label %tailrecurse.backedge, label %lab2.split]
+
+tailrecurse.backedge:
+  br label %tailrecurse
+
+lab2.split:
+  %asmresult5 = extractvalue { i32, i32 } %0, 1
+  ret i32 %asmresult5
+}

llvmbot · 2025-08-10T13:44:14Z

@llvm/pr-subscribers-llvm-selectiondag

Author: XChy (XChy)

Changes

Depend on #152591 to fix #149023.
Similar to an EH pad, there is no real advantage in "falling through" to an indirect target of an INLINEASM_BR. And multiple indirect targets of inline asm at the end of a function may be rotated infinitely.
Therefore, this patch avoids such optimization on indirect target of inline asm as fall through.

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

3 Files Affected:

(modified) llvm/lib/CodeGen/BranchFolding.cpp (+8-2)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+9-4)
(added) llvm/test/CodeGen/X86/callbr-asm-loop.ll (+41)

diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index dcfd9aad70fc5..d49c4b4a5755e 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -1776,8 +1776,9 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
       // Okay, there is no really great place to put this block.  If, however,
       // the block before this one would be a fall-through if this block were
       // removed, move this block to the end of the function. There is no real
-      // advantage in "falling through" to an EH block, so we don't want to
-      // perform this transformation for that case.
+      // advantage in "falling through" to an EH block or an indirect target of
+      // an INLINEASM_BR, so we don't want to perform this transformation for
+      // that case.
       //
       // Also, Windows EH introduced the possibility of an arbitrary number of
       // successors to a given block.  The analyzeBranch call does not consider
@@ -1787,10 +1788,15 @@ bool BranchFolder::OptimizeBlock(MachineBasicBlock *MBB) {
       // below were performed for EH "FallThrough" blocks.  Therefore, even if
       // that appears not to be happening anymore, we should assume that it is
       // possible and not remove the "!FallThrough()->isEHPad" condition below.
+      //
+      // And inline asm branches also introduced the possibility of infinite
+      // rotation, as there are an arbitrary number of successors to a given
+      // block.
       MachineBasicBlock *PrevTBB = nullptr, *PrevFBB = nullptr;
       SmallVector<MachineOperand, 4> PrevCond;
       if (FallThrough != MF.end() &&
           !FallThrough->isEHPad() &&
+          !FallThrough->isInlineAsmBrIndirectTarget() &&
           !TII->analyzeBranch(PrevBB, PrevTBB, PrevFBB, PrevCond, true) &&
           PrevBB.isSuccessor(&*FallThrough)) {
         MBB->moveAfter(&MF.back());
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index d0815e9f51822..f096148f865cb 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12737,17 +12737,22 @@ static Register FollowCopyChain(MachineRegisterInfo &MRI, Register Reg) {
   assert(MI->getOpcode() == TargetOpcode::COPY &&
          "start of copy chain MUST be COPY");
   Reg = MI->getOperand(1).getReg();
+
+  // If the copied register in the first copy must be virtual.
+  assert(Reg.isVirtual() && "expected COPY of virtual register");
   MI = MRI.def_begin(Reg)->getParent();
+
   // There may be an optional second copy.
   if (MI->getOpcode() == TargetOpcode::COPY) {
     assert(Reg.isVirtual() && "expected COPY of virtual register");
     Reg = MI->getOperand(1).getReg();
     assert(Reg.isPhysical() && "expected COPY of physical register");
-    MI = MRI.def_begin(Reg)->getParent();
+  } else {
+    // The start of the chain must be an INLINEASM_BR.
+    assert(MI->getOpcode() == TargetOpcode::INLINEASM_BR &&
+           "end of copy chain MUST be INLINEASM_BR");
   }
-  // The start of the chain must be an INLINEASM_BR.
-  assert(MI->getOpcode() == TargetOpcode::INLINEASM_BR &&
-         "end of copy chain MUST be INLINEASM_BR");
+
   return Reg;
 }
 
diff --git a/llvm/test/CodeGen/X86/callbr-asm-loop.ll b/llvm/test/CodeGen/X86/callbr-asm-loop.ll
new file mode 100644
index 0000000000000..9e890a861e3e4
--- /dev/null
+++ b/llvm/test/CodeGen/X86/callbr-asm-loop.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+
+; RUN: llc -O1 -mtriple=i686-- < %s | FileCheck %s
+
+; Test that causes multiple defs of %eax.
+; FIXME: The testcase hangs with -O1/2/3 enabled.
+define i32 @loop1() {
+; CHECK-LABEL: loop1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    .p2align 4
+; CHECK-NEXT:  .LBB0_1: # %tailrecurse
+; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    movl $1, %edx
+; CHECK-NEXT:    #APP
+; CHECK-NEXT:    #NO_APP
+; CHECK-NEXT:    jmp .LBB0_1
+; CHECK-NEXT:  .LBB0_2: # Inline asm indirect target
+; CHECK-NEXT:    # %tailrecurse.tailrecurse_crit_edge
+; CHECK-NEXT:    # in Loop: Header=BB0_1 Depth=1
+; CHECK-NEXT:    # Label of block must be emitted
+; CHECK-NEXT:    jmp .LBB0_1
+; CHECK-NEXT:  .LBB0_3: # Inline asm indirect target
+; CHECK-NEXT:    # %lab2.split
+; CHECK-NEXT:    # Label of block must be emitted
+; CHECK-NEXT:    movl %edx, %eax
+; CHECK-NEXT:    retl
+entry:
+  br label %tailrecurse
+
+tailrecurse:
+  %0 = callbr { i32, i32 } asm "", "={ax},={dx},0,1,!i,!i"(i32 0, i32 1) #1
+          to label %tailrecurse.backedge [label %tailrecurse.backedge, label %lab2.split]
+
+tailrecurse.backedge:
+  br label %tailrecurse
+
+lab2.split:
+  %asmresult5 = extractvalue { i32, i32 } %0, 1
+  ret i32 %asmresult5
+}

github-actions · 2025-08-10T13:46:54Z

✅ With the latest revision this PR passed the C/C++ code formatter.

dtcxzyw · 2025-08-10T16:07:38Z

llvm/test/CodeGen/X86/callbr-asm-loop.ll

+; RUN: llc -O1 -mtriple=i686-- < %s | FileCheck %s
+
+; Test that causes multiple defs of %eax.
+; FIXME: The testcase hangs with -O1/2/3 enabled.


The comment needs to be updated.

llvm/test/CodeGen/X86/callbr-asm-loop.ll

llvm/lib/CodeGen/BranchFolding.cpp

…rget

dtcxzyw

LGTM w/ the fixme removed.

llvm/lib/CodeGen/BranchFolding.cpp

XChy · 2025-08-14T05:58:32Z

Any other review? @arsenm

arsenm

Something feels slightly off about this; analyzeBranch is probably the worst API in llvm

XChy requested review from arsenm, nikic, RKSimon, dtcxzyw and topperc August 10, 2025 13:43

llvmbot added backend:X86 llvm:codegen llvm:SelectionDAG SelectionDAGISel as well labels Aug 10, 2025

dtcxzyw reviewed Aug 10, 2025

View reviewed changes

XChy requested a review from jyknight August 11, 2025 06:50

arsenm requested a review from nickdesaulniers August 12, 2025 07:10

arsenm reviewed Aug 12, 2025

View reviewed changes

llvm/test/CodeGen/X86/callbr-asm-loop.ll Show resolved Hide resolved

llvm/lib/CodeGen/BranchFolding.cpp Outdated Show resolved Hide resolved

XChy force-pushed the fix-branchfolding-inlineasmbr branch from f08762e to 8301034 Compare August 12, 2025 14:40

XChy changed the title ~~[BranchFolding] Avoid moving blocks to fall through to an indirect target of inline asm~~ [BranchFolding] Avoid moving blocks to fall through to an indirect target Aug 12, 2025

XChy force-pushed the fix-branchfolding-inlineasmbr branch from 8301034 to a9108cb Compare August 13, 2025 07:26

[BranchFolding] Avoid moving blocks to fall through to an indirect ta…

2913dc6

…rget

XChy force-pushed the fix-branchfolding-inlineasmbr branch from a9108cb to 2913dc6 Compare August 13, 2025 07:28

dtcxzyw approved these changes Aug 13, 2025

View reviewed changes

dtcxzyw reviewed Aug 13, 2025

View reviewed changes

llvm/lib/CodeGen/BranchFolding.cpp Outdated Show resolved Hide resolved

XChy added 2 commits August 14, 2025 01:34

fix comment

818ab09

Merge branch 'main' into fix-branchfolding-inlineasmbr

571a859

arsenm approved these changes Aug 14, 2025

View reviewed changes

arsenm merged commit f393f2a into llvm:main Aug 14, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[BranchFolding] Avoid moving blocks to fall through to an indirect target #152916

[BranchFolding] Avoid moving blocks to fall through to an indirect target #152916

XChy commented Aug 10, 2025

Uh oh!

llvmbot commented Aug 10, 2025

Uh oh!

llvmbot commented Aug 10, 2025

Uh oh!

github-actions bot commented Aug 10, 2025 •

edited

Loading

Uh oh!

dtcxzyw Aug 10, 2025

Uh oh!

Uh oh!

Uh oh!

dtcxzyw left a comment

Uh oh!

Uh oh!

XChy commented Aug 14, 2025

Uh oh!

arsenm left a comment

Uh oh!

Uh oh!

Uh oh!

[BranchFolding] Avoid moving blocks to fall through to an indirect target #152916

[BranchFolding] Avoid moving blocks to fall through to an indirect target #152916

Conversation

XChy commented Aug 10, 2025

Uh oh!

llvmbot commented Aug 10, 2025

Uh oh!

llvmbot commented Aug 10, 2025

Uh oh!

github-actions bot commented Aug 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtcxzyw Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

XChy commented Aug 14, 2025

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 10, 2025 •

edited

Loading