From 447eb33836b1a3a51a015c14f81522c46320f970 Mon Sep 17 00:00:00 2001 From: vikashgu Date: Wed, 12 Feb 2025 15:23:23 +0000 Subject: [PATCH 1/4] [AMDGPU][MachineRegisterInfo] Extend the MRI live-ins check by including the check against the subregs for live-ins. --- llvm/include/llvm/CodeGen/MachineRegisterInfo.h | 2 +- llvm/lib/CodeGen/MachineRegisterInfo.cpp | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h index 1c465741cb462..f182b42d2a24a 100644 --- a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h +++ b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h @@ -1020,7 +1020,7 @@ class MachineRegisterInfo { return LiveIns; } - bool isLiveIn(Register Reg) const; + bool isLiveIn(Register Reg, bool CheckForSubreg = false) const; /// getLiveInPhysReg - If VReg is a live-in virtual register, return the /// corresponding live-in physical register. diff --git a/llvm/lib/CodeGen/MachineRegisterInfo.cpp b/llvm/lib/CodeGen/MachineRegisterInfo.cpp index 937f63f6c5e00..341b0c7207092 100644 --- a/llvm/lib/CodeGen/MachineRegisterInfo.cpp +++ b/llvm/lib/CodeGen/MachineRegisterInfo.cpp @@ -447,10 +447,22 @@ void MachineRegisterInfo::clearKillFlags(Register Reg) const { MO.setIsKill(false); } -bool MachineRegisterInfo::isLiveIn(Register Reg) const { - for (const std::pair &LI : liveins()) +bool MachineRegisterInfo::isLiveIn(Register Reg, bool CheckForSubreg) const { + for (const std::pair &LI : liveins()) { if ((Register)LI.first == Reg || LI.second == Reg) return true; + + // Check if Reg is a subreg of live-in register + if (CheckForSubreg) { + MCRegister PhysReg = LI.first; + if (!PhysReg.isValid()) + continue; + + for (MCPhysReg SubReg : getTargetRegisterInfo()->subregs(PhysReg)) + if (SubReg == Reg) + return true; + } + } return false; } From e384137656621f9f09e4d8fad4c146e2d48dff4d Mon Sep 17 00:00:00 2001 From: vikashgu Date: Thu, 13 Feb 2025 07:17:45 +0000 Subject: [PATCH 2/4] Modified MRI liviIn check to have unconditional subreg check --- llvm/include/llvm/CodeGen/MachineRegisterInfo.h | 2 +- llvm/lib/CodeGen/MachineRegisterInfo.cpp | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h index f182b42d2a24a..1c465741cb462 100644 --- a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h +++ b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h @@ -1020,7 +1020,7 @@ class MachineRegisterInfo { return LiveIns; } - bool isLiveIn(Register Reg, bool CheckForSubreg = false) const; + bool isLiveIn(Register Reg) const; /// getLiveInPhysReg - If VReg is a live-in virtual register, return the /// corresponding live-in physical register. diff --git a/llvm/lib/CodeGen/MachineRegisterInfo.cpp b/llvm/lib/CodeGen/MachineRegisterInfo.cpp index 341b0c7207092..edac380dcf363 100644 --- a/llvm/lib/CodeGen/MachineRegisterInfo.cpp +++ b/llvm/lib/CodeGen/MachineRegisterInfo.cpp @@ -447,21 +447,19 @@ void MachineRegisterInfo::clearKillFlags(Register Reg) const { MO.setIsKill(false); } -bool MachineRegisterInfo::isLiveIn(Register Reg, bool CheckForSubreg) const { +bool MachineRegisterInfo::isLiveIn(Register Reg) const { for (const std::pair &LI : liveins()) { if ((Register)LI.first == Reg || LI.second == Reg) return true; // Check if Reg is a subreg of live-in register - if (CheckForSubreg) { - MCRegister PhysReg = LI.first; - if (!PhysReg.isValid()) - continue; + MCRegister PhysReg = LI.first; + if (!PhysReg.isValid()) + continue; - for (MCPhysReg SubReg : getTargetRegisterInfo()->subregs(PhysReg)) - if (SubReg == Reg) - return true; - } + for (MCPhysReg SubReg : getTargetRegisterInfo()->subregs(PhysReg)) + if (SubReg == Reg) + return true; } return false; } From df4bf0b14f7ab928890d745e6b2699f8dea9720d Mon Sep 17 00:00:00 2001 From: vg0204 Date: Tue, 25 Feb 2025 14:57:02 +0530 Subject: [PATCH 3/4] updated the pre-commit tests resolved via the extended MRI liveIns check. --- .../spill-partial-csr-sgpr-live-ins.mir | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/llvm/test/CodeGen/AMDGPU/spill-partial-csr-sgpr-live-ins.mir b/llvm/test/CodeGen/AMDGPU/spill-partial-csr-sgpr-live-ins.mir index ab960a7084528..834ae2f94538b 100644 --- a/llvm/test/CodeGen/AMDGPU/spill-partial-csr-sgpr-live-ins.mir +++ b/llvm/test/CodeGen/AMDGPU/spill-partial-csr-sgpr-live-ins.mir @@ -1,25 +1,6 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py -# 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 +# RUN: llc -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs -run-pass=si-lower-sgpr-spills -o - %s| FileCheck %s -# FIXME : Currently, MRI's liveIn check for registers does not take the corresponding live-in's sub-registers into account. As a result -# in SILowerSGPRSpills, the SubReg spill gets marked KILLED even though its SuperReg is in the function Live-ins. This causes machine -# verifier to now fail at direct usage of that SubReg, which intially should not be any problem before adding spill. - -# VERIFIER: After SI lower SGPR spill instructions - -# VERIFIER: *** Bad machine code: Using an undefined physical register *** -# VERIFIER: - instruction: S_NOP 0, implicit $sgpr50 -# VERIFIER-NEXT: - operand 1: implicit $sgpr50 - -# VERIFIER: *** Bad machine code: Using an undefined physical register *** -# VERIFIER: - instruction: S_NOP 0, implicit $sgpr52 -# VERIFIER-NEXT: - operand 1: implicit $sgpr52 - -# VERIFIER: *** Bad machine code: Using an undefined physical register *** -# VERIFIER: - instruction: S_NOP 0, implicit $sgpr55 -# VERIFIER-NEXT: - operand 1: implicit $sgpr55 - -# VERIFIER: LLVM ERROR: Found 3 machine code errors. --- name: spill_partial_live_csr_sgpr_test tracksRegLiveness: true @@ -30,7 +11,22 @@ liveins: body: | bb.0: liveins: $sgpr50_sgpr51, $sgpr52_sgpr53, $sgpr54_sgpr55 - + ; CHECK-LABEL: name: spill_partial_live_csr_sgpr_test + ; CHECK: liveins: $sgpr50, $sgpr52, $sgpr53, $sgpr54, $sgpr55, $sgpr56, $vgpr63, $sgpr50_sgpr51, $sgpr52_sgpr53, $sgpr54_sgpr55 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr50, 0, $vgpr63 + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr52, 1, $vgpr63 + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr53, 2, $vgpr63 + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr54, 3, $vgpr63 + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR $sgpr55, 4, $vgpr63 + ; CHECK-NEXT: $vgpr63 = SI_SPILL_S32_TO_VGPR killed $sgpr56, 5, $vgpr63 + ; CHECK-NEXT: S_NOP 0, implicit $sgpr50 + ; CHECK-NEXT: $sgpr50 = S_MOV_B32 0 + ; CHECK-NEXT: S_NOP 0, implicit $sgpr52 + ; CHECK-NEXT: $sgpr52_sgpr53 = S_MOV_B64 0 + ; CHECK-NEXT: S_NOP 0, implicit $sgpr55 + ; CHECK-NEXT: $sgpr54_sgpr55 = S_MOV_B64 0 + ; CHECK-NEXT: $sgpr56 = S_MOV_B32 0 S_NOP 0, implicit $sgpr50 $sgpr50 = S_MOV_B32 0 S_NOP 0, implicit $sgpr52 From 7b07a1a6314fb00466336cca409369ba0cdbadab Mon Sep 17 00:00:00 2001 From: vg0204 Date: Tue, 25 Feb 2025 15:07:45 +0530 Subject: [PATCH 4/4] Replaced subreg check with regUnits check in MRI isLiveIn --- llvm/lib/CodeGen/MachineRegisterInfo.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/MachineRegisterInfo.cpp b/llvm/lib/CodeGen/MachineRegisterInfo.cpp index edac380dcf363..93f4235cc7e97 100644 --- a/llvm/lib/CodeGen/MachineRegisterInfo.cpp +++ b/llvm/lib/CodeGen/MachineRegisterInfo.cpp @@ -454,12 +454,14 @@ bool MachineRegisterInfo::isLiveIn(Register Reg) const { // Check if Reg is a subreg of live-in register MCRegister PhysReg = LI.first; - if (!PhysReg.isValid()) + if (!PhysReg.isValid() || !Reg.isPhysical()) continue; - for (MCPhysReg SubReg : getTargetRegisterInfo()->subregs(PhysReg)) - if (SubReg == Reg) - return true; + const TargetRegisterInfo *TRI = getTargetRegisterInfo(); + if (all_of(TRI->regunits(Reg), [&](const MCRegUnit RegUnit) { + return llvm::is_contained(TRI->regunits(PhysReg), RegUnit); + })) + return true; } return false; }