Skip to content

Commit b454b04

Browse files
momchil-velikovmomchil-velikov
authored
[AArch64] Fix a compiler crash in MachineSink (#67705)
There were a couple of issues with maintaining register def/uses held in `MachineRegisterInfo`: * when an operand is changed from one register to another, the corresponding instruction must already be inserted into the function, or MRI won't be updated * when traversing the set of all uses of a register, that set must not change
1 parent b35f294 commit b454b04

File tree

2 files changed

+191
-17
lines changed

2 files changed

+191
-17
lines changed

‎llvm/lib/CodeGen/MachineSink.cpp

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ bool MachineSinking::PerformSinkAndFold(MachineInstr &MI,
461461
}
462462

463463
if (UseInst.getParent() != MI.getParent()) {
464-
// If the register class of the register we are replacingis a superset
464+
// If the register class of the register we are replacing is a superset
465465
// of any of the register classes of the operands of the materialized
466466
// instruction don't consider that live range extended.
467467
const TargetRegisterClass *RCS = MRI->getRegClass(Reg);
@@ -527,8 +527,8 @@ bool MachineSinking::PerformSinkAndFold(MachineInstr &MI,
527527
if (U.getInt())
528528
continue;
529529
MachineInstr *NewDbgMI = SinkDst->getMF()->CloneMachineInstr(DbgMI);
530-
NewDbgMI->getOperand(0).setReg(DstReg);
531530
SinkMBB.insertAfter(InsertPt, NewDbgMI);
531+
NewDbgMI->getOperand(0).setReg(DstReg);
532532
}
533533
} else {
534534
// Fold instruction into the addressing mode of a memory instruction.
@@ -538,33 +538,35 @@ bool MachineSinking::PerformSinkAndFold(MachineInstr &MI,
538538
SinkDst->eraseFromParent();
539539
}
540540

541-
MI.eraseFromParent();
542-
543-
// Collect instructions that need to be deleted (COPYs). We cannot delete them
544-
// while traversing register uses.
545-
SmallVector<MachineInstr *> CleanupInstrs;
541+
// Collect operands that need to be cleaned up because the registers no longer
542+
// exist (in COPYs and debug instructions). We cannot delete instructions or
543+
// clear operands while traversing register uses.
544+
SmallVector<MachineOperand *> Cleanup;
546545
Worklist.push_back(DefReg);
547546
while (!Worklist.empty()) {
548547
Register Reg = Worklist.pop_back_val();
549-
550548
for (MachineOperand &MO : MRI->use_operands(Reg)) {
551549
MachineInstr *U = MO.getParent();
552550
assert((U->isCopy() || U->isDebugInstr()) &&
553551
"Only debug uses and copies must remain");
554-
if (U->isCopy()) {
552+
if (U->isCopy())
555553
Worklist.push_back(U->getOperand(0).getReg());
556-
CleanupInstrs.push_back(U);
557-
} else {
558-
MO.setReg(0);
559-
MO.setSubReg(0);
560-
}
554+
Cleanup.push_back(&MO);
561555
}
562556
}
563557

564-
// Delete the dead COPYs.
565-
for (MachineInstr *Del : CleanupInstrs)
566-
Del->eraseFromParent();
558+
// Delete the dead COPYs and clear operands in debug instructions
559+
for (MachineOperand *MO : Cleanup) {
560+
MachineInstr *I = MO->getParent();
561+
if (I->isCopy()) {
562+
I->eraseFromParent();
563+
} else {
564+
MO->setReg(0);
565+
MO->setSubReg(0);
566+
}
567+
}
567568

569+
MI.eraseFromParent();
568570
return true;
569571
}
570572

‎llvm/test/CodeGen/AArch64/sink-and-fold-dbg-value-crash.mir

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
2+
# RUN: llc --aarch64-enable-sink-fold=true --run-pass=machine-sink %s -o - | FileCheck %s
3+
--- |
4+
source_filename = "x.ll"
5+
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
6+
target triple = "aarch64-linux"
7+
8+
%S = type <{ ptr, %T, i64, i64, i32, [4 x i8] }>
9+
%T = type { ptr, ptr, ptr, ptr, i64 }
10+
11+
define void @f(ptr %p, i1 %c0, i1 %c1) {
12+
entry:
13+
%a = getelementptr %S, ptr %p, i64 0, i32 1
14+
call void @llvm.dbg.value(metadata ptr %a, metadata !4, metadata !DIExpression()), !dbg !10
15+
br i1 %c0, label %if.then, label %if.end
16+
17+
if.then: ; preds = %entry
18+
%v0 = tail call ptr @g(ptr %a)
19+
br i1 %c1, label %exit, label %if.end
20+
21+
if.end: ; preds = %if.then, %entry
22+
%v1 = load i64, ptr %a, align 8
23+
br label %exit
24+
25+
exit: ; preds = %if.end, %if.then
26+
ret void
27+
}
28+
29+
declare ptr @g(ptr)
30+
31+
declare void @llvm.dbg.value(metadata, metadata, metadata) #0
32+
33+
attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
34+
35+
!llvm.dbg.cu = !{!0}
36+
!llvm.module.flags = !{!2, !3}
37+
38+
!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang ", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
39+
!1 = !DIFile(filename: "f.c", directory: "/usr/rms")
40+
!2 = !{i32 7, !"Dwarf Version", i32 4}
41+
!3 = !{i32 2, !"Debug Info Version", i32 3}
42+
!4 = !DILocalVariable(name: "x", arg: 1, scope: !5, file: !1, line: 2, type: !8)
43+
!5 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 2, type: !6, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !9)
44+
!6 = !DISubroutineType(types: !7)
45+
!7 = !{!8, !8}
46+
!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
47+
!9 = !{}
48+
!10 = !DILocation(line: 2, column: 11, scope: !5)
49+
50+
...
51+
---
52+
name: f
53+
alignment: 4
54+
exposesReturnsTwice: false
55+
legalized: false
56+
regBankSelected: false
57+
selected: false
58+
failedISel: false
59+
tracksRegLiveness: true
60+
hasWinCFI: false
61+
callsEHReturn: false
62+
callsUnwindInit: false
63+
hasEHCatchret: false
64+
hasEHScopes: false
65+
hasEHFunclets: false
66+
isOutlined: false
67+
debugInstrRef: false
68+
failsVerification: false
69+
tracksDebugUserValues: false
70+
registers:
71+
- { id: 0, class: gpr64all, preferred-register: '' }
72+
- { id: 1, class: gpr64common, preferred-register: '' }
73+
- { id: 2, class: gpr32, preferred-register: '' }
74+
- { id: 3, class: gpr32, preferred-register: '' }
75+
- { id: 4, class: gpr32, preferred-register: '' }
76+
- { id: 5, class: gpr64sp, preferred-register: '' }
77+
- { id: 6, class: gpr64all, preferred-register: '' }
78+
liveins:
79+
- { reg: '$x0', virtual-reg: '%1' }
80+
- { reg: '$w1', virtual-reg: '%2' }
81+
- { reg: '$w2', virtual-reg: '%3' }
82+
frameInfo:
83+
isFrameAddressTaken: false
84+
isReturnAddressTaken: false
85+
hasStackMap: false
86+
hasPatchPoint: false
87+
stackSize: 0
88+
offsetAdjustment: 0
89+
maxAlignment: 1
90+
adjustsStack: true
91+
hasCalls: true
92+
stackProtector: ''
93+
functionContext: ''
94+
maxCallFrameSize: 0
95+
cvBytesOfCalleeSavedRegisters: 0
96+
hasOpaqueSPAdjustment: false
97+
hasVAStart: false
98+
hasMustTailInVarArgFunc: false
99+
hasTailCall: false
100+
localFrameSize: 0
101+
savePoint: ''
102+
restorePoint: ''
103+
fixedStack: []
104+
stack: []
105+
entry_values: []
106+
callSites: []
107+
debugValueSubstitutions: []
108+
constants: []
109+
machineFunctionInfo: {}
110+
body: |
111+
; CHECK-LABEL: name: f
112+
; CHECK: bb.0.entry:
113+
; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
114+
; CHECK-NEXT: liveins: $x0, $w1, $w2
115+
; CHECK-NEXT: {{ $}}
116+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr32 = COPY $w2
117+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr32 = COPY $w1
118+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64common = COPY $x0
119+
; CHECK-NEXT: DBG_VALUE $noreg, $noreg, !4, !DIExpression(), debug-location !10
120+
; CHECK-NEXT: TBZW [[COPY1]], 0, %bb.2
121+
; CHECK-NEXT: B %bb.1
122+
; CHECK-NEXT: {{ $}}
123+
; CHECK-NEXT: bb.1.if.then:
124+
; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.2(0x40000000)
125+
; CHECK-NEXT: {{ $}}
126+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr32 = COPY [[COPY]]
127+
; CHECK-NEXT: ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
128+
; CHECK-NEXT: $x0 = ADDXri [[COPY2]], 8, 0
129+
; CHECK-NEXT: DBG_VALUE $x0, $noreg, !4, !DIExpression(), debug-location !10
130+
; CHECK-NEXT: BL @g, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
131+
; CHECK-NEXT: ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
132+
; CHECK-NEXT: TBNZW [[COPY3]], 0, %bb.3
133+
; CHECK-NEXT: B %bb.2
134+
; CHECK-NEXT: {{ $}}
135+
; CHECK-NEXT: bb.2.if.end:
136+
; CHECK-NEXT: successors: %bb.3(0x80000000)
137+
; CHECK-NEXT: {{ $}}
138+
; CHECK-NEXT: {{ $}}
139+
; CHECK-NEXT: bb.3.exit:
140+
; CHECK-NEXT: RET_ReallyLR
141+
bb.0.entry:
142+
successors: %bb.1(0x40000000), %bb.2(0x40000000)
143+
liveins: $x0, $w1, $w2
144+
145+
%3:gpr32 = COPY $w2
146+
%2:gpr32 = COPY $w1
147+
%1:gpr64common = COPY $x0
148+
%4:gpr32 = COPY %3
149+
%5:gpr64sp = ADDXri %1, 8, 0
150+
DBG_VALUE %5, $noreg, !4, !DIExpression(), debug-location !10
151+
%0:gpr64all = COPY %5
152+
TBZW %2, 0, %bb.2
153+
B %bb.1
154+
155+
bb.1.if.then:
156+
successors: %bb.3(0x40000000), %bb.2(0x40000000)
157+
158+
ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
159+
$x0 = COPY %0
160+
BL @g, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
161+
ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
162+
TBNZW %4, 0, %bb.3
163+
B %bb.2
164+
165+
bb.2.if.end:
166+
successors: %bb.3(0x80000000)
167+
168+
169+
bb.3.exit:
170+
RET_ReallyLR
171+
172+
...

0 commit comments

Comments
 (0)