Skip to content

Commit 5cf0fb4

Browse files
topperctopperc
authored
[StackSlotColoring] Ignore non-spill objects in RemoveDeadStores. (#80242)
The stack slot coloring pass is concerned with optimizing spill slots. If any change is a pass is made over the function to remove stack stores that use the same register and stack slot as an immediately preceding load. The register check is too simple for constant registers like AArch64 and RISC-V's zero register. This register can be used as the result of a load if we want to discard the result, but still have the memory access performed. Like for a volatile or atomic load. If the code sees a load from the zero register followed by a store of the zero register at the same stack slot, the pass mistakenly believes the store isn't needed. Since the main stack coloring optimization is only concerned with spill slots, it seems reasonable that RemoveDeadStores should only be concerned with spills. Since we never generate a reload of x0, this avoids the issue seen by RISC-V. Test case concept is adapted from pr30821.mir from X86. That test had to be updated to mark the stack slot as a spill slot. Fixes #80052.
1 parent edbd93d commit 5cf0fb4

File tree

3 files changed

+118
-2
lines changed

3 files changed

+118
-2
lines changed

‎llvm/lib/CodeGen/StackSlotColoring.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ bool StackSlotColoring::RemoveDeadStores(MachineBasicBlock* MBB) {
480480
if (!(StoreReg = TII->isStoreToStackSlot(*NextMI, SecondSS, StoreSize)))
481481
continue;
482482
if (FirstSS != SecondSS || LoadReg != StoreReg || FirstSS == -1 ||
483-
LoadSize != StoreSize)
483+
LoadSize != StoreSize || !MFI->isSpillSlotObjectIndex(FirstSS))
484484
continue;
485485

486486
++NumDead;

‎llvm/test/CodeGen/RISCV/pr80052.mir

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
2+
# RUN: llc %s -mtriple=riscv64 -run-pass=greedy,virtregrewriter,stack-slot-coloring -o - | FileCheck %s
3+
4+
---
5+
name: foo
6+
alignment: 4
7+
tracksRegLiveness: true
8+
frameInfo:
9+
maxAlignment: 4
10+
localFrameSize: 4
11+
stack:
12+
- { id: 0, size: 1, alignment: 4, local-offset: -4 }
13+
machineFunctionInfo:
14+
varArgsFrameIndex: 0
15+
varArgsSaveSize: 0
16+
body: |
17+
bb.0.entry:
18+
; To trick stack-slot-colouring to run its dead-store-elimination phase,
19+
; which is at fault, we need the register allocator to run, and spill in two
20+
; places that can have their slots merged. Achieve this by volatile-loading
21+
; data into all allocatable GPRs except $x31. Then, volatile storing them
22+
; later, leaving regalloc only $x31 to play with in the middle.
23+
$x1 = LW %stack.0, 0 :: (volatile load (s32))
24+
$x5 = LW %stack.0, 0 :: (volatile load (s32))
25+
$x6 = LW %stack.0, 0 :: (volatile load (s32))
26+
$x7 = LW %stack.0, 0 :: (volatile load (s32))
27+
$x8 = LW %stack.0, 0 :: (volatile load (s32))
28+
$x9 = LW %stack.0, 0 :: (volatile load (s32))
29+
$x10 = LW %stack.0, 0 :: (volatile load (s32))
30+
$x11 = LW %stack.0, 0 :: (volatile load (s32))
31+
$x12 = LW %stack.0, 0 :: (volatile load (s32))
32+
$x13 = LW %stack.0, 0 :: (volatile load (s32))
33+
$x14 = LW %stack.0, 0 :: (volatile load (s32))
34+
$x15 = LW %stack.0, 0 :: (volatile load (s32))
35+
$x16 = LW %stack.0, 0 :: (volatile load (s32))
36+
$x17 = LW %stack.0, 0 :: (volatile load (s32))
37+
$x18 = LW %stack.0, 0 :: (volatile load (s32))
38+
$x19 = LW %stack.0, 0 :: (volatile load (s32))
39+
$x20 = LW %stack.0, 0 :: (volatile load (s32))
40+
$x21 = LW %stack.0, 0 :: (volatile load (s32))
41+
$x22 = LW %stack.0, 0 :: (volatile load (s32))
42+
$x23 = LW %stack.0, 0 :: (volatile load (s32))
43+
$x24 = LW %stack.0, 0 :: (volatile load (s32))
44+
$x25 = LW %stack.0, 0 :: (volatile load (s32))
45+
$x26 = LW %stack.0, 0 :: (volatile load (s32))
46+
$x27 = LW %stack.0, 0 :: (volatile load (s32))
47+
$x28 = LW %stack.0, 0 :: (volatile load (s32))
48+
$x29 = LW %stack.0, 0 :: (volatile load (s32))
49+
$x30 = LW %stack.0, 0 :: (volatile load (s32))
50+
51+
; Force the first spill.
52+
%0:gpr = LW %stack.0, 0 :: (volatile load (s32))
53+
%1:gpr = LW %stack.0, 0 :: (volatile load (s32))
54+
SW %1, %stack.0, 0 :: (volatile store (s32))
55+
SW %0, %stack.0, 0 :: (volatile store (s32))
56+
57+
; CHECK: renamable $x31 = LW %stack.0, 0 :: (volatile load (s32))
58+
; CHECK-NEXT: SD killed renamable $x31, %stack.1, 0 :: (store (s64) into %stack.1)
59+
; CHECK-NEXT: renamable $x31 = LW %stack.0, 0 :: (volatile load (s32))
60+
; CHECK-NEXT: SW killed renamable $x31, %stack.0, 0 :: (volatile store (s32))
61+
; CHECK-NEXT: renamable $x31 = LD %stack.1, 0 :: (load (s64) from %stack.1)
62+
; CHECK-NEXT: SW killed renamable $x31, %stack.0, 0 :: (volatile store (s32))
63+
64+
; stack-slot-coloring doesn't know that a write to $x0 is discarded.
65+
dead $x0 = LW %stack.0, 0 :: (volatile load (s32))
66+
; This stores 0 rather than the result of the preceding load since $x0
67+
; is special.
68+
; We don't want this store to be deleted.
69+
SW $x0, %stack.0, 0 :: (volatile store (s32))
70+
71+
; CHECK-NEXT: dead $x0 = LW %stack.0, 0 :: (volatile load (s32))
72+
; CHECK-NEXT: SW $x0, %stack.0, 0 :: (volatile store (s32))
73+
74+
; Force a second spill
75+
%2:gpr = LW %stack.0, 0 :: (volatile load (s32))
76+
%3:gpr = LW %stack.0, 0 :: (volatile load (s32))
77+
SW %3, %stack.0, 0 :: (volatile store (s32))
78+
SW %2, %stack.0, 0 :: (volatile store (s32))
79+
80+
; CHECK-NEXT: renamable $x31 = LW %stack.0, 0 :: (volatile load (s32))
81+
; CHECK-NEXT: SD killed renamable $x31, %stack.1, 0 :: (store (s64) into %stack.1)
82+
; CHECK-NEXT: renamable $x31 = LW %stack.0, 0 :: (volatile load (s32))
83+
; CHECK-NEXT: SW killed renamable $x31, %stack.0, 0 :: (volatile store (s32))
84+
; CHECK-NEXT: renamable $x31 = LD %stack.1, 0 :: (load (s64) from %stack.1)
85+
; CHECK-NEXT: SW killed renamable $x31, %stack.0, 0 :: (volatile store (s32))
86+
87+
SW $x1, %stack.0, 0 :: (volatile store (s32))
88+
SW $x5, %stack.0, 0 :: (volatile store (s32))
89+
SW $x6, %stack.0, 0 :: (volatile store (s32))
90+
SW $x7, %stack.0, 0 :: (volatile store (s32))
91+
SW $x8, %stack.0, 0 :: (volatile store (s32))
92+
SW $x9, %stack.0, 0 :: (volatile store (s32))
93+
SW $x10, %stack.0, 0 :: (volatile store (s32))
94+
SW $x11, %stack.0, 0 :: (volatile store (s32))
95+
SW $x12, %stack.0, 0 :: (volatile store (s32))
96+
SW $x13, %stack.0, 0 :: (volatile store (s32))
97+
SW $x14, %stack.0, 0 :: (volatile store (s32))
98+
SW $x15, %stack.0, 0 :: (volatile store (s32))
99+
SW $x16, %stack.0, 0 :: (volatile store (s32))
100+
SW $x17, %stack.0, 0 :: (volatile store (s32))
101+
SW $x18, %stack.0, 0 :: (volatile store (s32))
102+
SW $x19, %stack.0, 0 :: (volatile store (s32))
103+
SW $x20, %stack.0, 0 :: (volatile store (s32))
104+
SW $x21, %stack.0, 0 :: (volatile store (s32))
105+
SW $x22, %stack.0, 0 :: (volatile store (s32))
106+
SW $x23, %stack.0, 0 :: (volatile store (s32))
107+
SW $x24, %stack.0, 0 :: (volatile store (s32))
108+
SW $x25, %stack.0, 0 :: (volatile store (s32))
109+
SW $x26, %stack.0, 0 :: (volatile store (s32))
110+
SW $x27, %stack.0, 0 :: (volatile store (s32))
111+
SW $x28, %stack.0, 0 :: (volatile store (s32))
112+
SW $x29, %stack.0, 0 :: (volatile store (s32))
113+
SW $x30, %stack.0, 0 :: (volatile store (s32))
114+
PseudoRET
115+
116+
...

‎llvm/test/CodeGen/X86/pr30821.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ stack:
5151
- { id: 1, name: foxtrot, type: default, offset: 0, size: 16, alignment: 16,
5252
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
5353
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
54-
- { id: 2, name: india, type: default, offset: 0, size: 16, alignment: 16,
54+
- { id: 2, name: india, type: spill-slot, offset: 0, size: 16, alignment: 16,
5555
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
5656
debug-info-variable: '', debug-info-expression: '',
5757
debug-info-location: '' }

0 commit comments

Comments
 (0)