Skip to content

Commit

Permalink
[StackSlotColoring] Ignore non-spill objects in RemoveDeadStores.
Browse files Browse the repository at this point in the history
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 llvm#80052.
  • Loading branch information
topperc committed Feb 1, 2024
1 parent 19f1916 commit 739901b
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/StackSlotColoring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ bool StackSlotColoring::RemoveDeadStores(MachineBasicBlock* MBB) {
if (!(StoreReg = TII->isStoreToStackSlot(*NextMI, SecondSS, StoreSize)))
continue;
if (FirstSS != SecondSS || LoadReg != StoreReg || FirstSS == -1 ||
LoadSize != StoreSize)
LoadSize != StoreSize || !MFI->isSpillSlotObjectIndex(FirstSS))
continue;

++NumDead;
Expand Down
6 changes: 5 additions & 1 deletion llvm/test/CodeGen/RISCV/pr80052.mir
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ body: |
; CHECK: renamable $x31 = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
; CHECK-NEXT: SD killed renamable $x31, %stack.1, 0 :: (store (s64) into %stack.1)
; CHECK-NEXT: renamable $x31 = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
; CHECK-NEXT: SW killed renamable $x31, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
; CHECK-NEXT: renamable $x31 = LD %stack.1, 0 :: (load (s64) from %stack.1)
; CHECK-NEXT: SW killed renamable $x31, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
Expand All @@ -76,7 +78,7 @@ body: |
SW $x0, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
; CHECK-NEXT: dead $x0 = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
; There should be a store of $x0 here.
; CHECK-NEXT: SW $x0, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
; Force a second spill
%2:gpr = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
Expand All @@ -86,6 +88,8 @@ body: |
; CHECK-NEXT: renamable $x31 = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
; CHECK-NEXT: SD killed renamable $x31, %stack.1, 0 :: (store (s64) into %stack.1)
; CHECK-NEXT: renamable $x31 = LW %stack.0.alpha, 0 :: (volatile load (s32) from %ir.alpha)
; CHECK-NEXT: SW killed renamable $x31, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
; CHECK-NEXT: renamable $x31 = LD %stack.1, 0 :: (load (s64) from %stack.1)
; CHECK-NEXT: SW killed renamable $x31, %stack.0.alpha, 0 :: (volatile store (s32) into %ir.alpha)
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/X86/pr30821.mir
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ stack:
- { id: 1, name: foxtrot, type: default, offset: 0, size: 16, alignment: 16,
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
- { id: 2, name: india, type: default, offset: 0, size: 16, alignment: 16,
- { id: 2, name: india, type: spill-slot, offset: 0, size: 16, alignment: 16,
stack-id: default, callee-saved-register: '', callee-saved-restored: true,
debug-info-variable: '', debug-info-expression: '',
debug-info-location: '' }
Expand Down

0 comments on commit 739901b

Please sign in to comment.