Skip to content

Commit 4513776

Browse files
vhscamposvhscampos
authored
[ARM][Thumb2] Mark BTI-clearing instructions as scheduling region boundaries (#87982)
Following #68313 this patch extends the idea to M-profile PACBTI. The Machine Scheduler can reorder instructions within a scheduling region depending on the scheduling policy set. If a BTI-clearing instruction happens to partake in one such region, it might be moved around, therefore ending up where it shouldn't. The solution is to mark all BTI-clearing instructions as scheduling region boundaries. This essentially means that they must not be part of any scheduling region, and as consequence never get moved: - PAC - PACBTI - BTI - SG Note that PAC isn't BTI-clearing, but it's replaced by PACBTI late in the compilation pipeline. As far as I know, currently it isn't possible to organically obtain code that's susceptible to the bug: - Instructions that write to SP are region boundaries. PAC seems to always be followed by the pushing of r12 to the stack, so essentially PAC is always by itself in a scheduling region. - CALL_BTI is expanded into a machine instruction bundle. Bundles are unpacked only after the last machine scheduler run. Thus setjmp and BTI can be separated only if someone deliberately run the scheduler once more. - The BTI insertion pass is run late in the pipeline, only after the last machine scheduling has run. So once again it can be reordered only if someone deliberately runs the scheduler again. Nevertheless, one can reasonably argue that we should prevent the bug in spite of the compiler not being able to produce the required conditions for it. If things change, the compiler will be robust against this issue. The tests written for this are contrived: bogus MIR instructions have been added adjacent to the BTI-clearing instructions in order to have them inside non-trivial scheduling regions.
1 parent b4e7b56 commit 4513776

File tree

3 files changed

+192
-0
lines changed

3 files changed

+192
-0
lines changed

‎llvm/lib/Target/ARM/Thumb2InstrInfo.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,25 @@ MachineInstr *Thumb2InstrInfo::commuteInstructionImpl(MachineInstr &MI,
286286
return ARMBaseInstrInfo::commuteInstructionImpl(MI, NewMI, OpIdx1, OpIdx2);
287287
}
288288

289+
bool Thumb2InstrInfo::isSchedulingBoundary(const MachineInstr &MI,
290+
const MachineBasicBlock *MBB,
291+
const MachineFunction &MF) const {
292+
// BTI clearing instructions shall not take part in scheduling regions as
293+
// they must stay in their intended place. Although PAC isn't BTI clearing,
294+
// it can be transformed into PACBTI after the pre-RA Machine Scheduling
295+
// has taken place, so its movement must also be restricted.
296+
switch (MI.getOpcode()) {
297+
case ARM::t2BTI:
298+
case ARM::t2PAC:
299+
case ARM::t2PACBTI:
300+
case ARM::t2SG:
301+
return true;
302+
default:
303+
break;
304+
}
305+
return ARMBaseInstrInfo::isSchedulingBoundary(MI, MBB, MF);
306+
}
307+
289308
void llvm::emitT2RegPlusImmediate(MachineBasicBlock &MBB,
290309
MachineBasicBlock::iterator &MBBI,
291310
const DebugLoc &dl, Register DestReg,

‎llvm/lib/Target/ARM/Thumb2InstrInfo.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ class Thumb2InstrInfo : public ARMBaseInstrInfo {
6868
unsigned OpIdx1,
6969
unsigned OpIdx2) const override;
7070

71+
bool isSchedulingBoundary(const MachineInstr &MI,
72+
const MachineBasicBlock *MBB,
73+
const MachineFunction &MF) const override;
74+
7175
private:
7276
void expandLoadStackGuard(MachineBasicBlock::iterator MI) const override;
7377
};

‎llvm/test/CodeGen/ARM/misched-branch-targets.mir

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# RUN: llc -o - -run-pass=machine-scheduler -misched=shuffle %s | FileCheck %s
2+
# RUN: llc -o - -run-pass=postmisched %s | FileCheck %s
3+
4+
# REQUIRES: asserts
5+
# -misched=shuffle is only available with assertions enabled
6+
7+
--- |
8+
target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
9+
target triple = "thumbv8.1m.main-arm-none-eabi"
10+
11+
define i32 @foo_bti() #0 {
12+
entry:
13+
ret i32 0
14+
}
15+
16+
define i32 @foo_pac() #0 {
17+
entry:
18+
ret i32 0
19+
}
20+
21+
define i32 @foo_pacbti() #0 {
22+
entry:
23+
ret i32 0
24+
}
25+
26+
define i32 @foo_setjmp() #0 {
27+
entry:
28+
ret i32 0
29+
if.then:
30+
ret i32 0
31+
}
32+
33+
define i32 @foo_sg() #0 {
34+
entry:
35+
ret i32 0
36+
}
37+
38+
declare i32 @setjmp(ptr noundef) #1
39+
declare void @longjmp(ptr noundef, i32 noundef) #2
40+
41+
attributes #0 = { "frame-pointer"="all" "target-cpu"="cortex-m55" "target-features"="+armv8.1-m.main" }
42+
attributes #1 = { nounwind returns_twice "frame-pointer"="all" "target-cpu"="cortex-m55" "target-features"="+armv8.1-m.main" }
43+
attributes #2 = { noreturn nounwind "frame-pointer"="all" "target-cpu"="cortex-m55" "target-features"="+armv8.1-m.main" }
44+
45+
...
46+
---
47+
name: foo_bti
48+
tracksRegLiveness: true
49+
body: |
50+
bb.0.entry:
51+
liveins: $r0
52+
53+
t2BTI
54+
renamable $r0, dead $cpsr = nsw tADDi8 killed renamable $r0, 1, 14 /* CC::al */, $noreg
55+
tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
56+
57+
...
58+
59+
# CHECK-LABEL: name: foo_bti
60+
# CHECK: body:
61+
# CHECK-NEXT: bb.0.entry:
62+
# CHECK-NEXT: liveins: $r0
63+
# CHECK-NEXT: {{^ +$}}
64+
# CHECK-NEXT: t2BTI
65+
66+
---
67+
name: foo_pac
68+
tracksRegLiveness: true
69+
body: |
70+
bb.0.entry:
71+
liveins: $r0, $lr, $r12
72+
73+
frame-setup t2PAC implicit-def $r12, implicit $lr, implicit $sp
74+
renamable $r2 = nsw t2ADDri $r0, 3, 14 /* CC::al */, $noreg, $noreg
75+
$sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r7, killed $lr
76+
$r7 = frame-setup tMOVr killed $sp, 14 /* CC::al */, $noreg
77+
early-clobber $sp = frame-setup t2STR_PRE killed $r12, $sp, -4, 14 /* CC::al */, $noreg
78+
$r12, $sp = frame-destroy t2LDR_POST $sp, 4, 14 /* CC::al */, $noreg
79+
$sp = frame-destroy t2LDMIA_UPD $sp, 14 /* CC::al */, $noreg, def $r7, def $lr
80+
t2AUT implicit $r12, implicit $lr, implicit $sp
81+
tBX_RET 14 /* CC::al */, $noreg, implicit $r0
82+
83+
...
84+
85+
# CHECK-LABEL: name: foo_pac
86+
# CHECK: body:
87+
# CHECK-NEXT: bb.0.entry:
88+
# CHECK-NEXT: liveins: $r0, $lr, $r12
89+
# CHECK-NEXT: {{^ +$}}
90+
# CHECK-NEXT: frame-setup t2PAC implicit-def $r12, implicit $lr, implicit $sp
91+
92+
---
93+
name: foo_pacbti
94+
tracksRegLiveness: true
95+
body: |
96+
bb.0.entry:
97+
liveins: $r0, $lr, $r12
98+
99+
frame-setup t2PACBTI implicit-def $r12, implicit $lr, implicit $sp
100+
renamable $r2 = nsw t2ADDri $r0, 3, 14 /* CC::al */, $noreg, $noreg
101+
$sp = frame-setup t2STMDB_UPD $sp, 14 /* CC::al */, $noreg, killed $r7, killed $lr
102+
$r7 = frame-setup tMOVr killed $sp, 14 /* CC::al */, $noreg
103+
early-clobber $sp = frame-setup t2STR_PRE killed $r12, $sp, -4, 14 /* CC::al */, $noreg
104+
$r12, $sp = frame-destroy t2LDR_POST $sp, 4, 14 /* CC::al */, $noreg
105+
$sp = frame-destroy t2LDMIA_UPD $sp, 14 /* CC::al */, $noreg, def $r7, def $lr
106+
t2AUT implicit $r12, implicit $lr, implicit $sp
107+
tBX_RET 14 /* CC::al */, $noreg, implicit $r0
108+
109+
...
110+
111+
# CHECK-LABEL: name: foo_pacbti
112+
# CHECK: body:
113+
# CHECK-NEXT: bb.0.entry:
114+
# CHECK-NEXT: liveins: $r0, $lr, $r12
115+
# CHECK-NEXT: {{^ +$}}
116+
# CHECK-NEXT: frame-setup t2PACBTI implicit-def $r12, implicit $lr, implicit $sp
117+
118+
---
119+
name: foo_setjmp
120+
tracksRegLiveness: true
121+
body: |
122+
bb.0.entry:
123+
successors: %bb.1
124+
liveins: $lr
125+
126+
frame-setup tPUSH 14 /* CC::al */, $noreg, $r7, killed $lr, implicit-def $sp, implicit $sp
127+
$r7 = frame-setup tMOVr $sp, 14 /* CC::al */, $noreg
128+
$sp = frame-setup tSUBspi $sp, 40, 14 /* CC::al */, $noreg
129+
renamable $r0 = tMOVr $sp, 14 /* CC::al */, $noreg
130+
tBL 14 /* CC::al */, $noreg, @setjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, implicit-def $r0
131+
t2BTI
132+
renamable $r2 = nsw t2ADDri $r0, 3, 14 /* CC::al */, $noreg, $noreg
133+
tCMPi8 killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
134+
t2IT 0, 2, implicit-def $itstate
135+
renamable $r0 = tMOVi8 $noreg, 0, 0 /* CC::eq */, $cpsr, implicit $itstate
136+
$sp = frame-destroy tADDspi $sp, 40, 0 /* CC::eq */, $cpsr, implicit $itstate
137+
frame-destroy tPOP_RET 0 /* CC::eq */, killed $cpsr, def $r7, def $pc, implicit killed $r0, implicit $sp, implicit killed $itstate
138+
139+
bb.1.if.then:
140+
renamable $r0 = tMOVr $sp, 14 /* CC::al */, $noreg
141+
renamable $r1, dead $cpsr = tMOVi8 1, 14 /* CC::al */, $noreg
142+
tBL 14 /* CC::al */, $noreg, @longjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit killed $r1, implicit-def $sp
143+
144+
...
145+
146+
# CHECK-LABEL: name: foo_setjmp
147+
# CHECK: body:
148+
# CHECK: tBL 14 /* CC::al */, $noreg, @setjmp, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, implicit-def $r0
149+
# CHECK-NEXT: t2BTI
150+
151+
---
152+
name: foo_sg
153+
tracksRegLiveness: true
154+
body: |
155+
bb.0.entry:
156+
liveins: $r0
157+
158+
t2SG 14 /* CC::al */, $noreg
159+
renamable $r0, dead $cpsr = nsw tADDi8 killed renamable $r0, 1, 14 /* CC::al */, $noreg
160+
tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
161+
162+
...
163+
164+
# CHECK-LABEL: name: foo_sg
165+
# CHECK: body:
166+
# CHECK-NEXT: bb.0.entry:
167+
# CHECK-NEXT: liveins: $r0
168+
# CHECK-NEXT: {{^ +$}}
169+
# CHECK-NEXT: t2SG

0 commit comments

Comments
 (0)