From 739901b7d771693f94a9cd14dbd71aaed1d22f5f Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 31 Jan 2024 20:41:23 -0800 Subject: [PATCH] [StackSlotColoring] Ignore non-spill objects in RemoveDeadStores. 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. --- llvm/lib/CodeGen/StackSlotColoring.cpp | 2 +- llvm/test/CodeGen/RISCV/pr80052.mir | 6 +++++- llvm/test/CodeGen/X86/pr30821.mir | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/StackSlotColoring.cpp b/llvm/lib/CodeGen/StackSlotColoring.cpp index c180f4d8f0365..6d3fc740b292a 100644 --- a/llvm/lib/CodeGen/StackSlotColoring.cpp +++ b/llvm/lib/CodeGen/StackSlotColoring.cpp @@ -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; diff --git a/llvm/test/CodeGen/RISCV/pr80052.mir b/llvm/test/CodeGen/RISCV/pr80052.mir index eb12968f39939..8006697c3bf27 100644 --- a/llvm/test/CodeGen/RISCV/pr80052.mir +++ b/llvm/test/CodeGen/RISCV/pr80052.mir @@ -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) @@ -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) @@ -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) diff --git a/llvm/test/CodeGen/X86/pr30821.mir b/llvm/test/CodeGen/X86/pr30821.mir index 9591d45e7baff..992ef8bbe55f0 100644 --- a/llvm/test/CodeGen/X86/pr30821.mir +++ b/llvm/test/CodeGen/X86/pr30821.mir @@ -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: '' }