Skip to content

Commit b91f239

Browse files
arichardsonarichardson
committed
MipsDelaySlotFiller: Don't move BUNDLE instructions into the delay slot
Summary: In our CHERI fork we use BUNDLE instructions to ensure that a three-instruction sequence to generate a program-counter-relative value is emitted without reordering or insertions (since that would break the 32-bit offset computation). This sequence is created in MipsExpandPseudo and we use finalizeBundle() to create the BUNDLE instruction. However, the delay slot filler currently breaks this pattern since the BUNDLE will be removed and so all instructions are moved into the delay slot. Since the delay slot only executes the first instruction, this results in incorrect computations (and run-time crashes) if the branch is taken. The original test cases uses CHERI instructions, so for the test case here I simple filled a BUNDLE with a no-op DADDiu $sp_64, -16 and DADDiu $sp_64, 16. Reviewers: atanasyan Reviewed By: atanasyan Subscribers: merge_guards_bot, sdardis, hiraditya, jrtc27, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D70944
1 parent 0cc4b95 commit b91f239

File tree

2 files changed

+148
-0
lines changed

2 files changed

+148
-0
lines changed

‎llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,12 @@ bool MipsDelaySlotFiller::searchRange(MachineBasicBlock &MBB, IterTy Begin,
695695
continue;
696696
}
697697

698+
if (CurrI->isBundle()) {
699+
LLVM_DEBUG(dbgs() << DEBUG_TYPE ": ignoring BUNDLE instruction: ";
700+
CurrI->dump());
701+
continue;
702+
}
703+
698704
if (terminateSearch(*CurrI)) {
699705
LLVM_DEBUG(dbgs() << DEBUG_TYPE ": should terminate search: ";
700706
CurrI->dump());

‎llvm/test/CodeGen/Mips/delay-slot-filler-bundled-insts.mir

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
## Check that the delay-slot filler does not attempt to split BUNDLE instructions
3+
# RUN: llc %s -start-before=mips-delay-slot-filler -stop-after=mips-delay-slot-filler \
4+
# RUN: -verify-machineinstrs -o - | FileCheck %s
5+
--- |
6+
target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
7+
target triple = "mips64-unknown-freebsd"
8+
declare i8* @func_a(i64 zeroext)
9+
declare i8* @func_b(i64 zeroext)
10+
; Function Attrs: nounwind
11+
define i8* @test(i64 zeroext %nbytes) local_unnamed_addr #0 {
12+
entry:
13+
%cmp = icmp eq i64 %nbytes, 0
14+
br i1 %cmp, label %if.else, label %if.then
15+
16+
if.then: ; preds = %entry
17+
%call = tail call i8* @func_a(i64 zeroext %nbytes)
18+
br label %return
19+
20+
if.else: ; preds = %entry
21+
%call1 = tail call i8* @func_b(i64 zeroext 0)
22+
br label %return
23+
24+
return: ; preds = %if.else, %if.then
25+
%retval.0 = phi i8* [ %call, %if.then ], [ %call1, %if.else ]
26+
ret i8* %retval.0
27+
}
28+
; Function Attrs: nounwind
29+
declare void @llvm.stackprotector(i8*, i8**) #0
30+
31+
attributes #0 = { nounwind }
32+
33+
...
34+
---
35+
name: test
36+
alignment: 8
37+
exposesReturnsTwice: false
38+
legalized: false
39+
regBankSelected: false
40+
selected: false
41+
failedISel: false
42+
tracksRegLiveness: true
43+
hasWinCFI: false
44+
registers: []
45+
liveins:
46+
- { reg: '$a0_64', virtual-reg: '' }
47+
frameInfo:
48+
isFrameAddressTaken: false
49+
isReturnAddressTaken: false
50+
hasStackMap: false
51+
hasPatchPoint: false
52+
stackSize: 16
53+
offsetAdjustment: 0
54+
maxAlignment: 8
55+
adjustsStack: true
56+
hasCalls: true
57+
stackProtector: ''
58+
maxCallFrameSize: 0
59+
cvBytesOfCalleeSavedRegisters: 0
60+
hasOpaqueSPAdjustment: false
61+
hasVAStart: false
62+
hasMustTailInVarArgFunc: false
63+
localFrameSize: 0
64+
savePoint: ''
65+
restorePoint: ''
66+
fixedStack: []
67+
stack:
68+
- { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8,
69+
stack-id: default, callee-saved-register: '$ra_64', callee-saved-restored: true,
70+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
71+
callSites: []
72+
constants: []
73+
machineFunctionInfo: {}
74+
body: |
75+
; CHECK-LABEL: name: test
76+
; CHECK: bb.0.entry:
77+
; CHECK: successors: %bb.2(0x30000000), %bb.1(0x50000000)
78+
; CHECK: $sp_64 = DADDiu $sp_64, -16
79+
; CHECK: CFI_INSTRUCTION def_cfa_offset 16
80+
; CHECK: SD killed $ra_64, $sp_64, 8 :: (store 8 into %stack.0)
81+
; CHECK: CFI_INSTRUCTION offset $ra_64, -8
82+
; CHECK: BUNDLE {
83+
; CHECK: $sp_64 = DADDiu $sp_64, -16
84+
; CHECK: $sp_64 = DADDiu $sp_64, 16
85+
; CHECK: }
86+
; CHECK: BEQ64 renamable $a0_64, $zero_64, %bb.2, implicit-def $at {
87+
; CHECK: NOP
88+
; CHECK: }
89+
; CHECK: bb.1.if.then:
90+
; CHECK: successors: %bb.3(0x80000000)
91+
; CHECK: JAL @func_a, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64 {
92+
; CHECK: NOP
93+
; CHECK: }
94+
; CHECK: J %bb.3, implicit-def dead $at {
95+
; CHECK: NOP
96+
; CHECK: }
97+
; CHECK: bb.2.if.else:
98+
; CHECK: successors: %bb.3(0x80000000)
99+
; CHECK: JAL @func_b, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64 {
100+
; CHECK: $a0_64 = DADDiu $zero_64, 0
101+
; CHECK: }
102+
; CHECK: bb.3.return:
103+
; CHECK: $ra_64 = LD $sp_64, 8 :: (load 8 from %stack.0)
104+
; CHECK: PseudoReturn64 undef $ra_64, implicit $v0_64 {
105+
; CHECK: $sp_64 = DADDiu $sp_64, 16
106+
; CHECK: }
107+
bb.0.entry:
108+
successors: %bb.2(0x30000000), %bb.1(0x50000000)
109+
liveins: $a0_64, $ra_64
110+
111+
$sp_64 = DADDiu $sp_64, -16
112+
CFI_INSTRUCTION def_cfa_offset 16
113+
SD killed $ra_64, $sp_64, 8 :: (store 8 into %stack.0)
114+
CFI_INSTRUCTION offset $ra_64, -8
115+
; This BUNDLE instruction must not be split by the delay slot filler:
116+
BUNDLE {
117+
$sp_64 = DADDiu $sp_64, -16
118+
$sp_64 = DADDiu $sp_64, 16
119+
}
120+
BEQ64 renamable $a0_64, $zero_64, %bb.2, implicit-def $at
121+
122+
bb.1.if.then:
123+
successors: %bb.3(0x80000000)
124+
liveins: $a0_64
125+
126+
JAL @func_a, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64
127+
J %bb.3, implicit-def dead $at
128+
129+
bb.2.if.else:
130+
successors: %bb.3(0x80000000)
131+
132+
$a0_64 = DADDiu $zero_64, 0
133+
JAL @func_b, csr_n64, implicit-def dead $ra, implicit $a0_64, implicit-def $sp, implicit-def $v0_64
134+
135+
bb.3.return:
136+
liveins: $v0_64
137+
138+
$ra_64 = LD $sp_64, 8 :: (load 8 from %stack.0)
139+
$sp_64 = DADDiu $sp_64, 16
140+
PseudoReturn64 undef $ra_64, implicit $v0_64
141+
142+
...

0 commit comments

Comments
 (0)