Skip to content

Commit bdb6320

Browse files
vg0204vg0204
authored
[AMDGPU][CodeGen] Using MBB's liveIn check in tandem with MCRegAliasIterator in SILowerSGPRSpills (#129848)
This patch replaces use of MachineRegisterInfo's liveIn check with the machine basicBlock's liveIn. As the MRI's liveIn is inconsistent with the entry MBB liveIns, when it comes to the machine verifier checks. PS: Its an alternative solution with respect to #126926.
1 parent 0813c5c commit bdb6320

File tree

3 files changed

+43
-39
lines changed

3 files changed

+43
-39
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,16 @@ 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+
104114
/// Insert spill code for the callee-saved registers used in the function.
105115
static void insertCSRSaves(MachineBasicBlock &SaveBlock,
106116
ArrayRef<CalleeSavedInfo> CSI, SlotIndexes *Indexes,
@@ -114,8 +124,6 @@ static void insertCSRSaves(MachineBasicBlock &SaveBlock,
114124

115125
MachineBasicBlock::iterator I = SaveBlock.begin();
116126
if (!TFI->spillCalleeSavedRegisters(SaveBlock, I, CSI, TRI)) {
117-
const MachineRegisterInfo &MRI = MF.getRegInfo();
118-
119127
for (const CalleeSavedInfo &CS : CSI) {
120128
// Insert the spill to the stack frame.
121129
MCRegister Reg = CS.getReg();
@@ -128,7 +136,7 @@ static void insertCSRSaves(MachineBasicBlock &SaveBlock,
128136
// incoming register value, so don't kill at the spill point. This happens
129137
// since we pass some special inputs (workgroup IDs) in the callee saved
130138
// range.
131-
const bool IsLiveIn = MRI.isLiveIn(Reg);
139+
const bool IsLiveIn = isLiveIntoMBB(Reg, SaveBlock, TRI);
132140
TII.storeRegToStackSlot(SaveBlock, I, Reg, !IsLiveIn, CS.getFrameIdx(),
133141
RC, TRI, Register());
134142

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

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,6 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
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
2+
# RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -run-pass=si-lower-sgpr-spills -o - %s | FileCheck %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.
234
---
245
name: spill_partial_live_csr_sgpr_test
256
tracksRegLiveness: true
@@ -31,6 +12,21 @@ body: |
3112
bb.0:
3213
liveins: $sgpr50_sgpr51, $sgpr52_sgpr53, $sgpr54_sgpr55
3314
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
3430
S_NOP 0, implicit $sgpr50
3531
$sgpr50 = S_MOV_B32 0
3632
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 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
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
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)