Skip to content

Commit f5992b8

Browse files
ronliebronlieb
committed
Revert "[AMDGPU][CodeGen] Using MBB's liveIn check in tandem with MCRegAliasIterator in SILowerSGPRSpills (llvm#129848)"
This reverts commit bdb6320.
1 parent 99f202f commit f5992b8

File tree

4 files changed

+46
-70
lines changed

4 files changed

+46
-70
lines changed

‎llvm/lib/CodeGen/LiveDebugValues/HeterogeneousImpl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ namespace {
5858
class HeterogeneousLDV : public LDVImpl {
5959
private:
6060
bool ExtendRanges(MachineFunction &MF, MachineDominatorTree *DomTree,
61-
TargetPassConfig *TPC, unsigned InputBBLimit,
61+
bool ShouldEmitDebugEntryValues, unsigned InputBBLimit,
6262
unsigned InputDbgValLimit) override;
6363

6464
public:
@@ -74,7 +74,7 @@ HeterogeneousLDV::~HeterogeneousLDV() {}
7474

7575
bool HeterogeneousLDV::ExtendRanges(MachineFunction &MF,
7676
MachineDominatorTree *DomTree,
77-
TargetPassConfig *TPC,
77+
bool ShouldEmitDebugEntryValues,
7878
unsigned InputBBLimit,
7979
unsigned InputDbgValLimit) {
8080
LLVM_DEBUG(dbgs() << "\nDebug Range Extension\n");

‎llvm/lib/Target/AMDGPU/SILowerSGPRSpills.cpp

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,6 @@ INITIALIZE_PASS_END(SILowerSGPRSpillsLegacy, DEBUG_TYPE,
101101

102102
char &llvm::SILowerSGPRSpillsLegacyID = SILowerSGPRSpillsLegacy::ID;
103103

104-
static bool isLiveIntoMBB(MCRegister Reg, MachineBasicBlock &MBB,
105-
const TargetRegisterInfo *TRI) {
106-
for (MCRegAliasIterator R(Reg, TRI, true); R.isValid(); ++R) {
107-
if (MBB.isLiveIn(*R)) {
108-
return true;
109-
}
110-
}
111-
return false;
112-
}
113-
114104
/// Insert spill code for the callee-saved registers used in the function.
115105
static void insertCSRSaves(const GCNSubtarget &ST, MachineBasicBlock &SaveBlock,
116106
ArrayRef<CalleeSavedInfo> CSI,
@@ -119,32 +109,14 @@ static void insertCSRSaves(const GCNSubtarget &ST, MachineBasicBlock &SaveBlock,
119109
const TargetFrameLowering *TFI = ST.getFrameLowering();
120110
const TargetRegisterInfo *TRI = ST.getRegisterInfo();
121111
MachineBasicBlock::iterator I = SaveBlock.begin();
122-
if (!TFI->spillCalleeSavedRegisters(SaveBlock, I, CSI, TRI)) {
123-
for (const CalleeSavedInfo &CS : CSI) {
124-
// Insert the spill to the stack frame.
125-
MCRegister Reg = CS.getReg();
126-
127-
MachineInstrSpan MIS(I, &SaveBlock);
128-
const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(
129-
Reg, Reg == RI->getReturnAddressReg(MF) ? MVT::i64 : MVT::i32);
130-
131-
// If this value was already livein, we probably have a direct use of the
132-
// incoming register value, so don't kill at the spill point. This happens
133-
// since we pass some special inputs (workgroup IDs) in the callee saved
134-
// range.
135-
const bool IsLiveIn = isLiveIntoMBB(Reg, SaveBlock, TRI);
136-
TII.storeRegToStackSlot(SaveBlock, I, Reg, !IsLiveIn, CS.getFrameIdx(),
137-
RC, TRI, Register());
138-
139-
if (Indexes) {
140-
assert(std::distance(MIS.begin(), I) == 1);
141-
MachineInstr &Inst = *std::prev(I);
142-
Indexes->insertMachineInstrInMaps(Inst);
143-
}
144-
145-
if (LIS)
146-
LIS->removeAllRegUnitsForPhysReg(Reg);
147-
}
112+
MachineInstrSpan MIS(I, &SaveBlock);
113+
bool Success = TFI->spillCalleeSavedRegisters(SaveBlock, I, CSI, TRI);
114+
assert(Success && "spillCalleeSavedRegisters should always succeed");
115+
(void)Success;
116+
117+
if (Indexes) {
118+
for (MachineInstr &Inst : make_range(MIS.begin(), I))
119+
Indexes->insertMachineInstrInMaps(Inst);
148120
}
149121
}
150122

‎llvm/test/CodeGen/AMDGPU/spill-partial-csr-sgpr-live-ins.mir

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2-
# RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -run-pass=si-lower-sgpr-spills -o - %s | FileCheck %s
2+
# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -run-pass=si-lower-sgpr-spills %s -o /dev/null 2>&1 | FileCheck -check-prefix=VERIFIER %s
33

4+
# FIXME : Currently, MRI's liveIn check for registers does not take the corresponding live-in's sub-registers into account. As a result
5+
# in SILowerSGPRSpills, the SubReg spill gets marked KILLED even though its SuperReg is in the function Live-ins. This causes machine
6+
# verifier to now fail at direct usage of that SubReg, which intially should not be any problem before adding spill.
7+
8+
# VERIFIER: After SI lower SGPR spill instructions
9+
10+
# VERIFIER: *** Bad machine code: Using an undefined physical register ***
11+
# VERIFIER: - instruction: S_NOP 0, implicit $sgpr50
12+
# VERIFIER-NEXT: - operand 1: implicit $sgpr50
13+
14+
# VERIFIER: *** Bad machine code: Using an undefined physical register ***
15+
# VERIFIER: - instruction: S_NOP 0, implicit $sgpr52
16+
# VERIFIER-NEXT: - operand 1: implicit $sgpr52
17+
18+
# VERIFIER: *** Bad machine code: Using an undefined physical register ***
19+
# VERIFIER: - instruction: S_NOP 0, implicit $sgpr55
20+
# VERIFIER-NEXT: - operand 1: implicit $sgpr55
21+
22+
# VERIFIER: LLVM ERROR: Found 3 machine code errors.
423
---
524
name: spill_partial_live_csr_sgpr_test
625
tracksRegLiveness: true
@@ -12,21 +31,6 @@ body: |
1231
bb.0:
1332
liveins: $sgpr50_sgpr51, $sgpr52_sgpr53, $sgpr54_sgpr55
1433
15-
; CHECK-LABEL: name: spill_partial_live_csr_sgpr_test
16-
; CHECK: liveins: $sgpr50, $sgpr52, $sgpr53, $sgpr54, $sgpr55, $vgpr63, $sgpr50_sgpr51, $sgpr52_sgpr53, $sgpr54_sgpr55
17-
; CHECK-NEXT: {{ $}}
18-
; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr50, 0, $vgpr63
19-
; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr52, 1, $vgpr63
20-
; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr53, 2, $vgpr63
21-
; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr54, 3, $vgpr63
22-
; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr55, 4, $vgpr63
23-
; CHECK-NEXT: S_NOP 0, implicit $sgpr50
24-
; CHECK-NEXT: $sgpr50 = S_MOV_B32 0
25-
; CHECK-NEXT: S_NOP 0, implicit $sgpr52
26-
; CHECK-NEXT: $sgpr52_sgpr53 = S_MOV_B64 0
27-
; CHECK-NEXT: S_NOP 0, implicit $sgpr55
28-
; CHECK-NEXT: $sgpr54_sgpr55 = S_MOV_B64 0
29-
; CHECK-NEXT: $sgpr56 = S_MOV_B32 0
3034
S_NOP 0, implicit $sgpr50
3135
$sgpr50 = S_MOV_B32 0
3236
S_NOP 0, implicit $sgpr52

‎llvm/test/CodeGen/AMDGPU/spill-sgpr-to-virtual-vgpr.mir

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,22 @@ body: |
5555
; GCN-LABEL: name: sgpr_spill_lane_crossover
5656
; GCN: liveins: $sgpr10, $sgpr64, $sgpr65, $sgpr66, $sgpr67, $sgpr68, $sgpr69, $sgpr70, $sgpr71, $sgpr80, $sgpr81, $sgpr82, $sgpr83, $sgpr84, $sgpr85, $sgpr86, $sgpr87, $vgpr63, $sgpr30_sgpr31, $sgpr64_sgpr65_sgpr66_sgpr67_sgpr68_sgpr69_sgpr70_sgpr71, $sgpr72_sgpr73_sgpr74_sgpr75_sgpr76_sgpr77_sgpr78_sgpr79, $sgpr80_sgpr81_sgpr82_sgpr83_sgpr84_sgpr85_sgpr86_sgpr87, $sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95
5757
; GCN-NEXT: {{ $}}
58-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr64, 0, $vgpr63
59-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr65, 1, $vgpr63
60-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr66, 2, $vgpr63
61-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr67, 3, $vgpr63
62-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr68, 4, $vgpr63
63-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr69, 5, $vgpr63
64-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr70, 6, $vgpr63
65-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr71, 7, $vgpr63
66-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr80, 8, $vgpr63
67-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr81, 9, $vgpr63
68-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr82, 10, $vgpr63
69-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr83, 11, $vgpr63
70-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr84, 12, $vgpr63
71-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr85, 13, $vgpr63
72-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr86, 14, $vgpr63
73-
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr87, 15, $vgpr63
58+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr64, 0, $vgpr63
59+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr65, 1, $vgpr63
60+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr66, 2, $vgpr63
61+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr67, 3, $vgpr63
62+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr68, 4, $vgpr63
63+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr69, 5, $vgpr63
64+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr70, 6, $vgpr63
65+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr71, 7, $vgpr63
66+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr80, 8, $vgpr63
67+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr81, 9, $vgpr63
68+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr82, 10, $vgpr63
69+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr83, 11, $vgpr63
70+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr84, 12, $vgpr63
71+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr85, 13, $vgpr63
72+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr86, 14, $vgpr63
73+
; GCN-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr87, 15, $vgpr63
7474
; GCN-NEXT: S_NOP 0
7575
; GCN-NEXT: [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
7676
; GCN-NEXT: [[DEF:%[0-9]+]]:vgpr_32 = SI_SPILL_S32_TO_VGPR killed $sgpr10, 0, [[DEF]]

0 commit comments

Comments
 (0)