Skip to content

Commit

Permalink
[llvm][IfConversion] update successor list when merging INLINEASM_BR
Browse files Browse the repository at this point in the history
If this successor list is not correct, then branch-folding may
incorrectly think that the indirect target is dead and remove it. This
results in a dangling reference to the removed block as an operand to
the INLINEASM_BR, which later will get AsmPrinted into code that doesn't
assemble.

This was made more obvious by, but is not a regression of
https://reviews.llvm.org/D130316.

Fixes: #60346

Reviewed By: efriedma, void

Differential Revision: https://reviews.llvm.org/D142924

(cherry picked from commit 07c7784)
  • Loading branch information
nickdesaulniers authored and tru committed Feb 9, 2023
1 parent af5fa49 commit d73eb82
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
9 changes: 9 additions & 0 deletions llvm/lib/CodeGen/IfConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2244,6 +2244,15 @@ void IfConverter::MergeBlocks(BBInfo &ToBBI, BBInfo &FromBBI, bool AddEdges) {
assert(!FromMBB.hasAddressTaken() &&
"Removing a BB whose address is taken!");

// If we're about to splice an INLINEASM_BR from FromBBI, we need to update
// ToBBI's successor list accordingly.
if (FromMBB.mayHaveInlineAsmBr())
for (MachineInstr &MI : FromMBB)
if (MI.getOpcode() == TargetOpcode::INLINEASM_BR)
for (MachineOperand &MO : MI.operands())
if (MO.isMBB() && !ToBBI.BB->isSuccessor(MO.getMBB()))
ToBBI.BB->addSuccessor(MO.getMBB(), BranchProbability::getZero());

// In case FromMBB contains terminators (e.g. return instruction),
// first move the non-terminator instructions, then the terminators.
MachineBasicBlock::iterator FromTI = FromMBB.getFirstTerminator();
Expand Down
19 changes: 14 additions & 5 deletions llvm/test/CodeGen/ARM/inlineasmbr-if-cvt.mir
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -start-before=if-converter -stop-after=if-converter -o - %s \
# RUN: | FileCheck %s
# RUN: -verify-machineinstrs | FileCheck %s
--- |
; ModuleID = 'reduced.ll'
source_filename = "reduced.ll"
Expand Down Expand Up @@ -80,14 +80,23 @@ machineFunctionInfo: {}
body: |
; CHECK-LABEL: name: tusb1210_chg_det_work
; CHECK: bb.0.entry:
; CHECK-NEXT: successors:
; CHECK-NEXT: successors: %bb.1(0x80000000), %bb.2(0x80000000)
; CHECK-NEXT: liveins: $r0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: TSTri killed renamable $r0, 1, 14 /* CC::al */, $noreg, implicit-def $cpsr
; CHECK-NEXT: renamable $r0 = MOVi 0, 1 /* CC::ne */, $cpsr, $noreg
; CHECK-NEXT: dead renamable $r0 = MOVi 1, 0 /* CC::eq */, killed $cpsr, $noreg, implicit killed $r0
; FIXME: %bb.-1 is a dangling reference!!!
; CHECK-NEXT: INLINEASM_BR &".word b, ${1:l}, ${0:c}\0A\09", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 0, 13 /* imm */, %bb.-1
; CHECK-NEXT: renamable $r0 = MOVi 1, 0 /* CC::eq */, killed $cpsr, $noreg, implicit killed $r0
; CHECK-NEXT: INLINEASM_BR &".word b, ${1:l}, ${0:c}\0A\09", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 0, 13 /* imm */, %bb.1
; CHECK-NEXT: B %bb.2
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1.if.end.sink.split (machine-block-address-taken, inlineasm-br-indirect-target):
; CHECK-NEXT: successors: %bb.2(0x80000000)
; CHECK-NEXT: liveins: $r0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $r1 = MOVi 0, 14 /* CC::al */, $noreg, $noreg
; CHECK-NEXT: STRi12 killed renamable $r0, killed renamable $r1, 0, 14 /* CC::al */, $noreg :: (store (s32) into `ptr null`)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2.if.end:
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg
bb.0.entry:
successors: %bb.4(0x40000000), %bb.1(0x40000000)
Expand Down

0 comments on commit d73eb82

Please sign in to comment.