Skip to content

Commit e48916f

Browse files
pzhengqcpzhengqc
authored
[ARM][ConstantIslands] Correct MinNoSplitDisp calculation (#114590)
MinNoSplitDisp was first introduced in D16890 to handle cases where the ConstantIslands pass fails to converge in the presence of big basic blocks. However, the computation of the variable seems to be wrong as it currently computes the offset immediately following UserBB. In other words, it represents the distance from the beginning of the function to the end of UserBB. The distance from the beginning of the function does not seem to be a good indicator of how big the basic block is unless the basic block is close to the beginning of the function. I think MinNoSplitDisp should compute the distance between UserOffset and the end of UserBB instead.
1 parent 0032c15 commit e48916f

File tree

2 files changed

+167
-1
lines changed

2 files changed

+167
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,8 @@ bool ARMConstantIslands::findAvailableWater(CPUser &U, unsigned UserOffset,
13231323
MachineBasicBlock *UserBB = U.MI->getParent();
13241324
BBInfoVector &BBInfo = BBUtils->getBBInfo();
13251325
const Align CPEAlign = getCPEAlign(U.CPEMI);
1326-
unsigned MinNoSplitDisp = BBInfo[UserBB->getNumber()].postOffset(CPEAlign);
1326+
unsigned MinNoSplitDisp =
1327+
BBInfo[UserBB->getNumber()].postOffset(CPEAlign) - UserOffset;
13271328
if (CloserWater && MinNoSplitDisp > U.getMaxDisp() / 2)
13281329
return false;
13291330
for (water_iterator IP = std::prev(WaterList.end()), B = WaterList.begin();;

‎llvm/test/CodeGen/Thumb2/constant-islands-no-split.mir

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
# RUN: llc -mtriple=thumbv7-linux-gnueabihf -run-pass=arm-cp-islands -arm-constant-island-max-iteration=1 %s -o - | FileCheck %s
2+
--- |
3+
; ModuleID = 'constant-islands-new-island.ll'
4+
source_filename = "constant-islands-new-island.ll"
5+
target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
6+
target triple = "thumbv7-unknown-linux-gnueabihf"
7+
8+
define void @test(i1 %tst) {
9+
entry:
10+
%0 = call i32 @llvm.arm.space(i32 2000, i32 undef)
11+
br label %smallbb
12+
13+
smallbb: ; preds = %entry
14+
br i1 %tst, label %true, label %false
15+
16+
true: ; preds = %false, %smallbb
17+
%val = phi float [ 1.234500e+04, %smallbb ], [ undef, %false ]
18+
%1 = call i32 @llvm.arm.space(i32 2000, i32 undef)
19+
call void @bar(float %val)
20+
ret void
21+
22+
false: ; preds = %smallbb
23+
br label %true
24+
}
25+
26+
declare void @bar(float)
27+
28+
; Function Attrs: nounwind
29+
declare i32 @llvm.arm.space(i32 immarg, i32) #0
30+
31+
attributes #0 = { nounwind }
32+
33+
...
34+
---
35+
name: test
36+
alignment: 2
37+
exposesReturnsTwice: false
38+
legalized: false
39+
regBankSelected: false
40+
selected: false
41+
failedISel: false
42+
tracksRegLiveness: true
43+
hasWinCFI: false
44+
noPhis: true
45+
isSSA: false
46+
noVRegs: true
47+
hasFakeUses: false
48+
callsEHReturn: false
49+
callsUnwindInit: false
50+
hasEHCatchret: false
51+
hasEHScopes: false
52+
hasEHFunclets: false
53+
isOutlined: false
54+
debugInstrRef: false
55+
failsVerification: false
56+
tracksDebugUserValues: false
57+
registers: []
58+
liveins:
59+
- { reg: '$r0', virtual-reg: '' }
60+
frameInfo:
61+
isFrameAddressTaken: false
62+
isReturnAddressTaken: false
63+
hasStackMap: false
64+
hasPatchPoint: false
65+
stackSize: 16
66+
offsetAdjustment: 0
67+
maxAlignment: 4
68+
adjustsStack: true
69+
hasCalls: true
70+
stackProtector: ''
71+
functionContext: ''
72+
maxCallFrameSize: 0
73+
cvBytesOfCalleeSavedRegisters: 0
74+
hasOpaqueSPAdjustment: false
75+
hasVAStart: false
76+
hasMustTailInVarArgFunc: false
77+
hasTailCall: false
78+
isCalleeSavedInfoValid: true
79+
localFrameSize: 0
80+
savePoint: ''
81+
restorePoint: ''
82+
fixedStack: []
83+
stack:
84+
- { id: 0, name: '', type: spill-slot, offset: -12, size: 4, alignment: 4,
85+
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
86+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
87+
- { id: 1, name: '', type: spill-slot, offset: -16, size: 4, alignment: 4,
88+
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
89+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
90+
- { id: 2, name: '', type: spill-slot, offset: -4, size: 4, alignment: 4,
91+
stack-id: default, callee-saved-register: '$lr', callee-saved-restored: false,
92+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
93+
- { id: 3, name: '', type: spill-slot, offset: -8, size: 4, alignment: 4,
94+
stack-id: default, callee-saved-register: '$r7', callee-saved-restored: true,
95+
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
96+
entry_values: []
97+
callSites: []
98+
debugValueSubstitutions: []
99+
constants:
100+
- id: 0
101+
value: 'float 1.234500e+04'
102+
alignment: 4
103+
isTargetSpecific: false
104+
machineFunctionInfo:
105+
isLRSpilled: true
106+
body: |
107+
bb.0.entry:
108+
successors: %bb.1(0x80000000)
109+
liveins: $r0, $r7, $lr
110+
111+
frame-setup tPUSH 14 /* CC::al */, $noreg, killed $r7, killed $lr, implicit-def $sp, implicit $sp
112+
frame-setup CFI_INSTRUCTION def_cfa_offset 8
113+
frame-setup CFI_INSTRUCTION offset $lr, -4
114+
frame-setup CFI_INSTRUCTION offset $r7, -8
115+
$sp = frame-setup tSUBspi $sp, 2, 14 /* CC::al */, $noreg
116+
frame-setup CFI_INSTRUCTION def_cfa_offset 16
117+
tSTRspi killed $r0, $sp, 1, 14 /* CC::al */, $noreg :: (store (s32) into %stack.0)
118+
renamable $r0 = IMPLICIT_DEF
119+
dead renamable $r0 = SPACE 2000, killed renamable $r0
120+
t2B %bb.1, 14 /* CC::al */, $noreg
121+
122+
bb.1.smallbb:
123+
successors: %bb.2(0x40000000), %bb.3(0x40000000)
124+
125+
$r0 = tLDRspi $sp, 1, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
126+
renamable $s0 = VLDRS %const.0, 0, 14 /* CC::al */, $noreg :: (load (s32) from constant-pool)
127+
renamable $r0, dead $cpsr = tLSLri renamable $r0, 31, 14 /* CC::al */, $noreg
128+
tCMPi8 killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
129+
VSTRS killed $s0, $sp, 0, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
130+
t2Bcc %bb.3, 0 /* CC::eq */, killed $cpsr
131+
t2B %bb.2, 14 /* CC::al */, $noreg
132+
133+
bb.2.true:
134+
$s0 = VLDRS $sp, 0, 14 /* CC::al */, $noreg :: (load (s32) from %stack.1)
135+
renamable $r0 = IMPLICIT_DEF
136+
dead renamable $r0 = SPACE 2000, killed renamable $r0
137+
tBL 14 /* CC::al */, $noreg, @bar, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $s0, implicit-def $sp
138+
$sp = frame-destroy tADDspi $sp, 2, 14 /* CC::al */, $noreg
139+
frame-destroy tPOP_RET 14 /* CC::al */, $noreg, def $r7, def $pc
140+
141+
bb.3.false:
142+
successors: %bb.2(0x80000000)
143+
144+
renamable $s0 = IMPLICIT_DEF
145+
t2B %bb.2, 14 /* CC::al */, $noreg
146+
147+
...
148+
# Check that smallbb is not split by the constant islands pass. Previously,
149+
# smallbb was split due to incorrect calculation of MinNoSplitDisp.
150+
#
151+
# CHECK: bb.1.smallbb:
152+
# CHECK-NEXT: successors: %bb.3(0x40000000), %bb.4(0x40000000)
153+
# CHECK-NEXT: {{^ $}}
154+
# CHECK-NEXT: $r0 = tLDRspi $sp, 1, 14 /* CC::al */, $noreg :: (load (s32) from %stack.0)
155+
# CHECK-NEXT: renamable $s0 = VLDRS %const.1, 0, 14 /* CC::al */, $noreg :: (load (s32) from constant-pool)
156+
# CHECK-NEXT: renamable $r0, dead $cpsr = tLSLri renamable $r0, 31, 14 /* CC::al */, $noreg
157+
# CHECK-NEXT: tCMPi8 killed renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
158+
# CHECK-NEXT: VSTRS killed $s0, $sp, 0, 14 /* CC::al */, $noreg :: (store (s32) into %stack.1)
159+
# CHECK-NEXT: t2Bcc %bb.4, 0 /* CC::eq */, killed $cpsr
160+
# CHECK-NEXT: tB %bb.3, 14 /* CC::al */, $noreg
161+
# CHECK-NEXT: {{^ $}}
162+
# CHECK-NEXT: bb.2 (align 4):
163+
# CHECK-NEXT: successors:
164+
# CHECK-NEXT: {{^ $}}
165+
# CHECK-NEXT: CONSTPOOL_ENTRY 1, %const.0, 4

0 commit comments

Comments
 (0)