[TailDuplicator] Determine if computed gotos using `blockaddress` #132536

dianqk · 2025-03-22T10:07:23Z

Using blockaddress should be more reliable than determining if an operand comes from a jump table index.

Alternative: Add the MachineInstr::MIFlag::ComputedGoto flag when lowering indirectbr. But I don't think this approach is suitable to backport.

I can confirm that Ajla and CPython are both fine.

llvmbot · 2025-03-22T10:08:02Z

@llvm/pr-subscribers-backend-x86

Author: dianqk (dianqk)

Changes

Using blockaddress should be more reliable than determining if an operand comes from a jump table index.

Alternative: Add the MachineInstr::MIFlag::ComputedGoto flag when lowering indirectbr. But I don't think this approach is suitable to backport.

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

3 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineInstr.h (+4-3)
(modified) llvm/lib/CodeGen/MachineInstr.cpp (+6)
(modified) llvm/test/CodeGen/X86/tail-dup-computed-goto.mir (+164-10)

diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index b26cabe801ee8..6651431c9afad 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -997,9 +997,7 @@ class MachineInstr
   bool isIndirectBranch(QueryType Type = AnyInBundle,
                         bool IncludeJumpTable = true) const {
     return hasProperty(MCID::IndirectBranch, Type) &&
-           (IncludeJumpTable || !llvm::any_of(operands(), [](const auto &Op) {
-              return Op.isJTI();
-            }));
+           (IncludeJumpTable || jumpToIRBlockAddressTaken());
   }
 
   bool isComputedGoto(QueryType Type = AnyInBundle) const {
@@ -2088,6 +2086,9 @@ class MachineInstr
                     MCSymbol *PreInstrSymbol, MCSymbol *PostInstrSymbol,
                     MDNode *HeapAllocMarker, MDNode *PCSections,
                     uint32_t CFIType, MDNode *MMRAs);
+
+  /// Returns true if all successors are IRBlockAddressTaken.
+  bool jumpToIRBlockAddressTaken() const;
 };
 
 /// Special DenseMapInfo traits to compare MachineInstr* by *value* of the
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 471666568e79a..3c2d7d2b9ddbd 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -359,6 +359,12 @@ void MachineInstr::setExtraInfo(MachineFunction &MF,
     Info.set<EIIK_MMO>(MMOs[0]);
 }
 
+bool MachineInstr::jumpToIRBlockAddressTaken() const {
+  return llvm::all_of(getParent()->successors(), [](const auto *Succ) {
+    return Succ->isIRBlockAddressTaken();
+  });
+}
+
 void MachineInstr::dropMemRefs(MachineFunction &MF) {
   if (memoperands_empty())
     return;
diff --git a/llvm/test/CodeGen/X86/tail-dup-computed-goto.mir b/llvm/test/CodeGen/X86/tail-dup-computed-goto.mir
index a472dc67d8d51..4d50094d606b9 100644
--- a/llvm/test/CodeGen/X86/tail-dup-computed-goto.mir
+++ b/llvm/test/CodeGen/X86/tail-dup-computed-goto.mir
@@ -8,9 +8,21 @@
   declare i64 @f3()
   declare i64 @f4()
   declare i64 @f5()
-  @computed_goto.dispatch = external global [5 x ptr]
-  define void @computed_goto() { ret void }
+  @computed_goto.dispatch = constant [5 x ptr] [ptr null, ptr blockaddress(@computed_goto, %bb1), ptr blockaddress(@computed_goto, %bb2), ptr blockaddress(@computed_goto, %bb3), ptr blockaddress(@computed_goto, %bb4)]
+  define void @computed_goto() {
+    start:
+      ret void
+    bb1:
+      ret void
+    bb2:
+      ret void
+    bb3:
+      ret void
+    bb4:
+      ret void
+  }
   define void @jump_table() { ret void }
+  define void @jump_table_pic() { ret void }
 ...
 ---
 name:            computed_goto
@@ -28,7 +40,7 @@ body:             |
   ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gr64_nosp = COPY [[COPY1]]
   ; CHECK-NEXT:   JMP64m $noreg, 8, [[COPY1]], @computed_goto.dispatch, $noreg
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT: bb.1.bb1 (ir-block-address-taken %ir-block.bb1):
   ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x20000000), %bb.3(0x20000000), %bb.4(0x20000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -39,7 +51,7 @@ body:             |
   ; CHECK-NEXT:   [[COPY5:%[0-9]+]]:gr64_nosp = COPY [[COPY4]]
   ; CHECK-NEXT:   JMP64m $noreg, 8, [[COPY4]], @computed_goto.dispatch, $noreg
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT: bb.2.bb2 (ir-block-address-taken %ir-block.bb2):
   ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x20000000), %bb.3(0x20000000), %bb.4(0x20000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -50,7 +62,7 @@ body:             |
   ; CHECK-NEXT:   [[COPY8:%[0-9]+]]:gr64_nosp = COPY [[COPY7]]
   ; CHECK-NEXT:   JMP64m $noreg, 8, [[COPY7]], @computed_goto.dispatch, $noreg
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT: bb.3.bb3 (ir-block-address-taken %ir-block.bb3):
   ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x20000000), %bb.3(0x20000000), %bb.4(0x20000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -61,7 +73,7 @@ body:             |
   ; CHECK-NEXT:   [[COPY11:%[0-9]+]]:gr64_nosp = COPY [[COPY10]]
   ; CHECK-NEXT:   JMP64m $noreg, 8, [[COPY10]], @computed_goto.dispatch, $noreg
   ; CHECK-NEXT: {{  $}}
-  ; CHECK-NEXT: bb.4:
+  ; CHECK-NEXT: bb.4.bb4 (ir-block-address-taken %ir-block.bb4):
   ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x20000000), %bb.3(0x20000000), %bb.4(0x20000000)
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -79,7 +91,7 @@ body:             |
     %0:gr64 = COPY %6
     JMP_1 %bb.5
 
-  bb.1:
+  bb.1.bb1 (ir-block-address-taken %ir-block.bb1):
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
     CALL64pcrel32 target-flags(x86-plt) @f1, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
     ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -87,7 +99,7 @@ body:             |
     %1:gr64 = COPY %10
     JMP_1 %bb.5
 
-  bb.2:
+  bb.2.bb2 (ir-block-address-taken %ir-block.bb2):
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
     CALL64pcrel32 target-flags(x86-plt) @f2, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
     ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -95,7 +107,7 @@ body:             |
     %2:gr64 = COPY %9
     JMP_1 %bb.5
 
-  bb.3:
+  bb.3.bb3 (ir-block-address-taken %ir-block.bb3):
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
     CALL64pcrel32 target-flags(x86-plt) @f3, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
     ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -103,7 +115,7 @@ body:             |
     %3:gr64 = COPY %8
     JMP_1 %bb.5
 
-  bb.4:
+  bb.4.bb4 (ir-block-address-taken %ir-block.bb4):
     ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
     CALL64pcrel32 target-flags(x86-plt) @f4, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
     ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
@@ -253,3 +265,145 @@ body:             |
   bb.7:
 
 ...
+---
+name:            jump_table_pic
+tracksRegLiveness: true
+jumpTable:
+  kind:            block-address
+  entries:
+    - id:              0
+      blocks:          [ '%bb.2', '%bb.3', '%bb.4', '%bb.5', '%bb.6' ]
+body:             |
+  ; CHECK-LABEL: name: jump_table_pic
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f0, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gr64 = COPY [[COPY]]
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.3(0x1999999a), %bb.4(0x1999999a), %bb.5(0x1999999a), %bb.6(0x1999999a), %bb.7(0x1999999a)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gr64 = PHI [[COPY1]], %bb.0, %3, %bb.7, %4, %bb.6, %5, %bb.5, %6, %bb.4, %7, %bb.3
+  ; CHECK-NEXT:   [[DEC64r:%[0-9]+]]:gr64_nosp = DEC64r [[PHI]], implicit-def dead $eflags
+  ; CHECK-NEXT:   [[LEA64r:%[0-9]+]]:gr64 = LEA64r $rip, 1, $noreg, %jump-table.0, $noreg
+  ; CHECK-NEXT:   [[MOVSX64rm32_:%[0-9]+]]:gr64 = MOVSX64rm32 [[DEC64r]], 4, [[DEC64r]], 0, $noreg :: (load (s32) from jump-table)
+  ; CHECK-NEXT:   [[ADD64rr:%[0-9]+]]:gr64 = ADD64rr [[LEA64r]], [[MOVSX64rm32_]], implicit-def dead $eflags
+  ; CHECK-NEXT:   JMP64r [[ADD64rr]]
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f1, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY3:%[0-9]+]]:gr64 = COPY [[COPY2]]
+  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f2, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY5:%[0-9]+]]:gr64 = COPY [[COPY4]]
+  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.5:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f3, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY6:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY7:%[0-9]+]]:gr64 = COPY [[COPY6]]
+  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.6:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f4, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY8:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY9:%[0-9]+]]:gr64 = COPY [[COPY8]]
+  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.7:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @f5, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+  ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+  ; CHECK-NEXT:   [[COPY10:%[0-9]+]]:gr64 = COPY $rax
+  ; CHECK-NEXT:   [[COPY11:%[0-9]+]]:gr64 = COPY [[COPY10]]
+  ; CHECK-NEXT:   JMP_1 %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.8:
+  bb.0:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f0, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %7:gr64 = COPY $rax
+    %0:gr64 = COPY %7
+
+  bb.1:
+    %1:gr64 = PHI %0, %bb.0, %6, %bb.6, %5, %bb.5, %4, %bb.4, %3, %bb.3, %2, %bb.2
+    %8:gr64_nosp = DEC64r %1, implicit-def dead $eflags
+
+  bb.8:
+    successors: %bb.2(0x1999999a), %bb.3(0x1999999a), %bb.4(0x1999999a), %bb.5(0x1999999a), %bb.6(0x1999999a)
+
+    %14:gr64 = LEA64r $rip, 1, $noreg, %jump-table.0, $noreg
+    %15:gr64 = MOVSX64rm32 %8, 4, %8, 0, $noreg :: (load (s32) from jump-table)
+    %16:gr64 = ADD64rr %14, killed %15, implicit-def dead $eflags
+    JMP64r killed %16
+
+  bb.2:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f1, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %13:gr64 = COPY $rax
+    %2:gr64 = COPY %13
+    JMP_1 %bb.1
+
+  bb.3:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f2, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %12:gr64 = COPY $rax
+    %3:gr64 = COPY %12
+    JMP_1 %bb.1
+
+  bb.4:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f3, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %11:gr64 = COPY $rax
+    %4:gr64 = COPY %11
+    JMP_1 %bb.1
+
+  bb.5:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f4, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %10:gr64 = COPY $rax
+    %5:gr64 = COPY %10
+    JMP_1 %bb.1
+
+  bb.6:
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    CALL64pcrel32 target-flags(x86-plt) @f5, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %9:gr64 = COPY $rax
+    %6:gr64 = COPY %9
+    JMP_1 %bb.1
+
+  bb.7:
+
+...

alexfh

I verified that this fixes the problems we've found so far, but I can't say anything about the correctness or other aspects of this approach.

alexfh · 2025-03-24T11:52:35Z

Please ensure the buildkite errors are not related to this PR.

llvm/lib/CodeGen/MachineInstr.cpp

arsenm

isIndirectBranch is a low level instruction query. I expect it to inform of properties that directly apply to the instruction itself, and it is inappropriate for it to inspect the CFG. This code should be moved somewhere else

dianqk · 2025-03-25T11:55:04Z

isIndirectBranch is a low level instruction query. I expect it to inform of properties that directly apply to the instruction itself, and it is inappropriate for it to inspect the CFG. This code should be moved somewhere else

I moved this to MachineBasicBlock.

alexfh · 2025-03-26T12:43:36Z

@arsenm @fhahn @nikic do you feel like this review is going to converge today? If not, I think, the right approach is to commit the revert first - #132431 and then reland the original commit together with this fix rather than keeping LLVM trunk in effectively a broken state.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

arsenm · 2025-03-26T12:45:54Z

llvm/test/CodeGen/X86/tail-dup-computed-goto.mir

+    %7:gr64 = COPY $rax
+    %0:gr64 = COPY %7


Could compact the register numbers, these will be renumbered on MIR parse which can be confusing when later debugging the testacy

dianqk · 2025-03-26T13:28:56Z

/cherry-pick 66f158d

llvmbot · 2025-03-26T13:34:57Z

/pull-request #133082

llvm-ci · 2025-03-26T13:35:36Z

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/18918

Here is the relevant piece of the build log for the reference

Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-shell :: Register/arm-gp-read.test (2793 of 2802)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/quit.test (2794 of 2802)
UNSUPPORTED: lldb-shell :: Process/Windows/process_load.cpp (2795 of 2802)
PASS: lldb-unit :: Utility/./UtilityTests/94/128 (2796 of 2802)
PASS: lldb-api :: api/multithreaded/TestMultithreaded.py (2797 of 2802)
PASS: lldb-api :: terminal/TestEditlineCompletions.py (2798 of 2802)
PASS: lldb-api :: commands/process/attach/TestProcessAttach.py (2799 of 2802)
PASS: lldb-api :: repl/clang/TestClangREPL.py (2800 of 2802)
PASS: lldb-api :: tools/lldb-dap/progress/TestDAP_Progress.py (2801 of 2802)
UNRESOLVED: lldb-api :: iohandler/sigint/TestIOHandlerPythonREPLSigint.py (2802 of 2802)
******************** TEST 'lldb-api :: iohandler/sigint/TestIOHandlerPythonREPLSigint.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/iohandler/sigint -p TestIOHandlerPythonREPLSigint.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 66f158d91803875de63d8f2a437ce8ecb22c4141)
  clang revision 66f158d91803875de63d8f2a437ce8ecb22c4141
  llvm revision 66f158d91803875de63d8f2a437ce8ecb22c4141
(lldb) settings clear -all
(lldb) settings set symbols.enable-external-lookup false
(lldb) settings set target.inherit-tcc true
(lldb) settings set target.disable-aslr false
(lldb) settings set target.detach-on-error false
(lldb) settings set target.auto-apply-fixits false
(lldb) settings set plugin.process.gdb-remote.packet-timeout 60
(lldb) settings set symbols.clang-modules-cache-path "/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api"
(lldb) settings set use-color false
(lldb) script -l python --
script -l python --
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> import time; print('running' + 'now'); time.sleep(10000);
import time; print('running' + 'now'); time.sleep(10000);
runningnow�
^Csettings set interpreter.prompt-on-quit false
quit
settings set interpreter.prompt-on-quit false^Jquit^JSkipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/iohandler/sigint
runCmd: settings clear -all

output:

…vm#132536) Using `blockaddress` should be more reliable than determining if an operand comes from a jump table index. Alternative: Add the `MachineInstr::MIFlag::ComputedGoto` flag when lowering `indirectbr`. But I don't think this approach is suitable to backport. (cherry picked from commit 66f158d)

…(NFC)

…vm#132536) Using `blockaddress` should be more reliable than determining if an operand comes from a jump table index. Alternative: Add the `MachineInstr::MIFlag::ComputedGoto` flag when lowering `indirectbr`. But I don't think this approach is suitable to backport. (cherry picked from commit 66f158d)

dianqk requested review from nikic, fhahn and alexfh March 22, 2025 10:07

llvmbot added the backend:X86 label Mar 22, 2025

This was referenced Mar 22, 2025

[TailDuplicator] Do not restrict the computed gotos #114990

Merged

Revert "[TailDuplicator] Do not restrict the computed gotos" #132431

Closed

nikic requested a review from arsenm March 22, 2025 15:12

alexfh approved these changes Mar 24, 2025

View reviewed changes

dianqk added 2 commits March 24, 2025 20:58

Add test cases

33b40ec

[TailDuplicator] Determine if computed gotos using blockaddress

4ed0b5f

dianqk force-pushed the computed-goto-blockaddress branch from 10353d6 to 4ed0b5f Compare March 24, 2025 12:58

arsenm reviewed Mar 24, 2025

View reviewed changes

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

Address feedback

6275a07

arsenm requested changes Mar 25, 2025

View reviewed changes

Address feedback

6e3eef9

arsenm approved these changes Mar 26, 2025

View reviewed changes

dianqk added 2 commits March 26, 2025 21:14

Compact the register numbers by register-coalescer

246f4a0

Document terminatorIsComputedGoto

c4204e2

dianqk merged commit 66f158d into llvm:main Mar 26, 2025
5 of 7 checks passed

dianqk added this to the LLVM 20.X Release milestone Mar 26, 2025

github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 26, 2025

github-project-automation bot added this to LLVM Release Status Mar 26, 2025

llvmbot moved this from Needs Triage to Done in LLVM Release Status Mar 26, 2025

dianqk added a commit that referenced this pull request Apr 2, 2025

[MachineInstr] Remove the code that was accidentally added in #132536 …

842785a

…(NFC)

dianqk deleted the computed-goto-blockaddress branch April 2, 2025 12:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TailDuplicator] Determine if computed gotos using `blockaddress` #132536

[TailDuplicator] Determine if computed gotos using `blockaddress` #132536

Uh oh!

dianqk commented Mar 22, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 22, 2025

Uh oh!

alexfh left a comment

Uh oh!

alexfh commented Mar 24, 2025

Uh oh!

Uh oh!

arsenm left a comment

Uh oh!

dianqk commented Mar 25, 2025

Uh oh!

alexfh commented Mar 26, 2025

Uh oh!

Uh oh!

arsenm Mar 26, 2025

Uh oh!

Uh oh!

dianqk commented Mar 26, 2025

Uh oh!

llvmbot commented Mar 26, 2025

Uh oh!

llvm-ci commented Mar 26, 2025

Uh oh!

Uh oh!

[TailDuplicator] Determine if computed gotos using blockaddress #132536

[TailDuplicator] Determine if computed gotos using blockaddress #132536

Uh oh!

Conversation

dianqk commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 22, 2025

Uh oh!

alexfh left a comment

Choose a reason for hiding this comment

Uh oh!

alexfh commented Mar 24, 2025

Uh oh!

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

dianqk commented Mar 25, 2025

Uh oh!

alexfh commented Mar 26, 2025

Uh oh!

Uh oh!

arsenm Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dianqk commented Mar 26, 2025

Uh oh!

llvmbot commented Mar 26, 2025

Uh oh!

llvm-ci commented Mar 26, 2025

Uh oh!

Uh oh!

[TailDuplicator] Determine if computed gotos using `blockaddress` #132536

[TailDuplicator] Determine if computed gotos using `blockaddress` #132536

dianqk commented Mar 22, 2025 •

edited

Loading