[AMDGPU] Implement CFI for CSR spills #164724
Open
+22,312
−12,206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce new SPILL pseudos to allow CFI to be generated for only CSR spills, and to make ISA-instruction-level accurate information. Other targets either generate slightly incorrect information or rely on conventions for how spills are placed within the entry block. The approach in this change produces larger unwind tables, with the increased size being spent on additional DW_CFA_advance_location instructions needed to describe the unwinding accurately. Co-authored-by: Scott Linder <scott.linder@amd.com> Co-authored-by: Venkata Ramanaiah Nalamothu <VenkataRamanaiah.Nalamothu@amd.com>
This was referenced Oct 22, 2025
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Oct 22, 2025
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-globalisel Author: Scott Linder (slinder1) ChangesIntroduce new SPILL pseudos to allow CFI to be generated for only CSR Other targets either generate slightly incorrect information or rely on Co-authored-by: Scott Linder <scott.linder@amd.com> Patch is 4.01 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164724.diff 101 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index 5a0b1afbdfdff..bbde3c49f64c6 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -2244,17 +2244,49 @@ bool SIFrameLowering::allocateScavengingFrameIndexesNearIncomingSP(
return true;
}
+static bool isLiveIntoMBB(MCRegister Reg, MachineBasicBlock &MBB,
+ const TargetRegisterInfo *TRI) {
+ for (MCRegAliasIterator R(Reg, TRI, true); R.isValid(); ++R) {
+ if (MBB.isLiveIn(*R)) {
+ return true;
+ }
+ }
+ return false;
+}
+
bool SIFrameLowering::spillCalleeSavedRegisters(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MI,
ArrayRef<CalleeSavedInfo> CSI, const TargetRegisterInfo *TRI) const {
MachineFunction *MF = MBB.getParent();
const GCNSubtarget &ST = MF->getSubtarget<GCNSubtarget>();
- if (!ST.useVGPRBlockOpsForCSR())
- return false;
+ const SIInstrInfo *TII = ST.getInstrInfo();
+ const SIRegisterInfo *SITRI = static_cast<const SIRegisterInfo *>(TRI);
+
+ if (!ST.useVGPRBlockOpsForCSR()) {
+ for (const CalleeSavedInfo &CS : CSI) {
+ // Insert the spill to the stack frame.
+ unsigned Reg = CS.getReg();
+
+ if (CS.isSpilledToReg()) {
+ BuildMI(MBB, MI, DebugLoc(), TII->get(TargetOpcode::COPY),
+ CS.getDstReg())
+ .addReg(Reg, getKillRegState(true));
+ } else {
+ const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(
+ Reg, Reg == SITRI->getReturnAddressReg(*MF) ? MVT::i64 : MVT::i32);
+ // If this value was already livein, we probably have a direct use of
+ // the incoming register value, so don't kill at the spill point. This
+ // happens since we pass some special inputs (workgroup IDs) in the
+ // callee saved range.
+ const bool IsLiveIn = isLiveIntoMBB(Reg, MBB, TRI);
+ TII->storeRegToStackSlotCFI(MBB, MI, Reg, !IsLiveIn, CS.getFrameIdx(),
+ RC, TRI);
+ }
+ }
+ return true;
+ }
MachineFrameInfo &FrameInfo = MF->getFrameInfo();
- SIMachineFunctionInfo *MFI = MF->getInfo<SIMachineFunctionInfo>();
- const SIInstrInfo *TII = ST.getInstrInfo();
SIMachineFunctionInfo *FuncInfo = MF->getInfo<SIMachineFunctionInfo>();
const TargetRegisterClass *BlockRegClass =
@@ -2278,10 +2310,10 @@ bool SIFrameLowering::spillCalleeSavedRegisters(
FrameInfo.getObjectAlign(FrameIndex));
BuildMI(MBB, MI, MI->getDebugLoc(),
- TII->get(AMDGPU::SI_BLOCK_SPILL_V1024_SAVE))
+ TII->get(AMDGPU::SI_BLOCK_SPILL_V1024_CFI_SAVE))
.addReg(Reg, getKillRegState(false))
.addFrameIndex(FrameIndex)
- .addReg(MFI->getStackPtrOffsetReg())
+ .addReg(FuncInfo->getStackPtrOffsetReg())
.addImm(0)
.addImm(Mask)
.addMemOperand(MMO);
@@ -2467,6 +2499,22 @@ MachineInstr *SIFrameLowering::buildCFI(MachineBasicBlock &MBB,
.setMIFlag(flag);
}
+MachineInstr *SIFrameLowering::buildCFIForVRegToVRegSpill(
+ MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, const Register Reg, const Register RegCopy) const {
+ MachineFunction &MF = *MBB.getParent();
+ const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
+ const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
+
+ unsigned MaskReg = MCRI.getDwarfRegNum(
+ ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC, false);
+ auto CFIInst = MCCFIInstruction::createLLVMVectorRegisterMask(
+ nullptr, MCRI.getDwarfRegNum(Reg, false),
+ MCRI.getDwarfRegNum(RegCopy, false), VGPRLaneBitSize, MaskReg,
+ ST.getWavefrontSize());
+ return buildCFI(MBB, MBBI, DL, std::move(CFIInst));
+}
+
MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
const DebugLoc &DL, const Register SGPR, const Register VGPR,
@@ -2515,6 +2563,34 @@ MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
return buildCFI(MBB, MBBI, DL, std::move(CFIInst));
}
+MachineInstr *SIFrameLowering::buildCFIForSGPRToVMEMSpill(
+ MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, unsigned SGPR, int64_t Offset) const {
+ MachineFunction &MF = *MBB.getParent();
+ const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
+ return buildCFI(MBB, MBBI, DL,
+ llvm::MCCFIInstruction::createOffset(
+ nullptr, MCRI.getDwarfRegNum(SGPR, false), Offset));
+}
+
+MachineInstr *SIFrameLowering::buildCFIForVGPRToVMEMSpill(
+ MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, unsigned VGPR, int64_t Offset) const {
+ const MachineFunction &MF = *MBB.getParent();
+ const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
+ const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
+
+ int DwarfVGPR = MCRI.getDwarfRegNum(VGPR, false);
+ assert(DwarfVGPR != -1);
+
+ unsigned MaskReg = MCRI.getDwarfRegNum(
+ ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC, false);
+ auto CFIInst = MCCFIInstruction::createLLVMVectorOffset(
+ nullptr, DwarfVGPR, VGPRLaneBitSize, MaskReg, ST.getWavefrontSize(),
+ Offset);
+ return buildCFI(MBB, MBBI, DL, std::move(CFIInst));
+}
+
MachineInstr *SIFrameLowering::buildCFIForRegToSGPRPairSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
const DebugLoc &DL, const Register Reg, const Register SGPRPair) const {
@@ -2535,3 +2611,14 @@ MachineInstr *SIFrameLowering::buildCFIForRegToSGPRPairSpill(
nullptr, DwarfReg, DwarfSGPR0, SGPRBitSize, DwarfSGPR1, SGPRBitSize);
return buildCFI(MBB, MBBI, DL, std::move(CFIInst));
}
+
+MachineInstr *
+SIFrameLowering::buildCFIForSameValue(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, Register Reg) const {
+ const MachineFunction &MF = *MBB.getParent();
+ const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
+ int DwarfReg = MCRI.getDwarfRegNum(Reg, /*isEH=*/false);
+ auto CFIInst = MCCFIInstruction::createSameValue(nullptr, DwarfReg);
+ return buildCFI(MBB, MBBI, DL, std::move(CFIInst));
+}
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.h b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
index 20f608f2dfc24..2b716db0b7a22 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.h
@@ -120,6 +120,13 @@ class SIFrameLowering final : public AMDGPUFrameLowering {
const DebugLoc &DL, const MCCFIInstruction &CFIInst,
MachineInstr::MIFlag flag = MachineInstr::FrameSetup) const;
+ /// Create a CFI index describing a spill of the VGPR/AGPR \p Reg to another
+ /// VGPR/AGPR \p RegCopy and build a MachineInstr around it.
+ MachineInstr *buildCFIForVRegToVRegSpill(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL,
+ const Register Reg,
+ const Register RegCopy) const;
/// Create a CFI index describing a spill of an SGPR to a single lane of
/// a VGPR and build a MachineInstr around it.
MachineInstr *buildCFIForSGPRToVGPRSpill(MachineBasicBlock &MBB,
@@ -134,10 +141,25 @@ class SIFrameLowering final : public AMDGPUFrameLowering {
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
const DebugLoc &DL, Register SGPR,
ArrayRef<SIRegisterInfo::SpilledReg> VGPRSpills) const;
+ /// Create a CFI index describing a spill of a SGPR to VMEM and
+ /// build a MachineInstr around it.
+ MachineInstr *buildCFIForSGPRToVMEMSpill(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, unsigned SGPR,
+ int64_t Offset) const;
+ /// Create a CFI index describing a spill of a VGPR to VMEM and
+ /// build a MachineInstr around it.
+ MachineInstr *buildCFIForVGPRToVMEMSpill(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, unsigned VGPR,
+ int64_t Offset) const;
MachineInstr *buildCFIForRegToSGPRPairSpill(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
const DebugLoc &DL, Register Reg,
Register SGPRPair) const;
+ MachineInstr *buildCFIForSameValue(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MBBI,
+ const DebugLoc &DL, Register Reg) const;
// Returns true if the function may need to reserve space on the stack for the
// CWSR trap handler.
bool mayReserveScratchForCWSR(const MachineFunction &MF) const;
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index d930a21c2d7f5..a097e721d142f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -1530,22 +1530,26 @@ SIInstrInfo::getIndirectRegWriteMovRelPseudo(unsigned VecSize, unsigned EltSize,
return get(getIndirectVGPRWriteMovRelPseudoOpc(VecSize));
}
-static unsigned getSGPRSpillSaveOpcode(unsigned Size) {
+static unsigned getSGPRSpillSaveOpcode(unsigned Size, bool NeedsCFI) {
switch (Size) {
case 4:
- return AMDGPU::SI_SPILL_S32_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S32_CFI_SAVE : AMDGPU::SI_SPILL_S32_SAVE;
case 8:
- return AMDGPU::SI_SPILL_S64_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S64_CFI_SAVE : AMDGPU::SI_SPILL_S64_SAVE;
case 12:
- return AMDGPU::SI_SPILL_S96_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S96_CFI_SAVE : AMDGPU::SI_SPILL_S96_SAVE;
case 16:
- return AMDGPU::SI_SPILL_S128_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S128_CFI_SAVE
+ : AMDGPU::SI_SPILL_S128_SAVE;
case 20:
- return AMDGPU::SI_SPILL_S160_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S160_CFI_SAVE
+ : AMDGPU::SI_SPILL_S160_SAVE;
case 24:
- return AMDGPU::SI_SPILL_S192_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S192_CFI_SAVE
+ : AMDGPU::SI_SPILL_S192_SAVE;
case 28:
- return AMDGPU::SI_SPILL_S224_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S224_CFI_SAVE
+ : AMDGPU::SI_SPILL_S224_SAVE;
case 32:
return AMDGPU::SI_SPILL_S256_SAVE;
case 36:
@@ -1557,69 +1561,90 @@ static unsigned getSGPRSpillSaveOpcode(unsigned Size) {
case 48:
return AMDGPU::SI_SPILL_S384_SAVE;
case 64:
- return AMDGPU::SI_SPILL_S512_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S512_CFI_SAVE
+ : AMDGPU::SI_SPILL_S512_SAVE;
case 128:
- return AMDGPU::SI_SPILL_S1024_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_S1024_CFI_SAVE
+ : AMDGPU::SI_SPILL_S1024_SAVE;
default:
llvm_unreachable("unknown register size");
}
}
-static unsigned getVGPRSpillSaveOpcode(unsigned Size) {
+static unsigned getVGPRSpillSaveOpcode(unsigned Size, bool NeedsCFI) {
switch (Size) {
case 2:
return AMDGPU::SI_SPILL_V16_SAVE;
case 4:
- return AMDGPU::SI_SPILL_V32_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V32_CFI_SAVE : AMDGPU::SI_SPILL_V32_SAVE;
case 8:
- return AMDGPU::SI_SPILL_V64_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V64_CFI_SAVE : AMDGPU::SI_SPILL_V64_SAVE;
case 12:
- return AMDGPU::SI_SPILL_V96_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V96_CFI_SAVE : AMDGPU::SI_SPILL_V96_SAVE;
case 16:
- return AMDGPU::SI_SPILL_V128_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V128_CFI_SAVE
+ : AMDGPU::SI_SPILL_V128_SAVE;
case 20:
- return AMDGPU::SI_SPILL_V160_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V160_CFI_SAVE
+ : AMDGPU::SI_SPILL_V160_SAVE;
case 24:
- return AMDGPU::SI_SPILL_V192_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V192_CFI_SAVE
+ : AMDGPU::SI_SPILL_V192_SAVE;
case 28:
- return AMDGPU::SI_SPILL_V224_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V224_CFI_SAVE
+ : AMDGPU::SI_SPILL_V224_SAVE;
case 32:
- return AMDGPU::SI_SPILL_V256_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V256_CFI_SAVE
+ : AMDGPU::SI_SPILL_V256_SAVE;
case 36:
- return AMDGPU::SI_SPILL_V288_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V288_CFI_SAVE
+ : AMDGPU::SI_SPILL_V288_SAVE;
case 40:
- return AMDGPU::SI_SPILL_V320_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V320_CFI_SAVE
+ : AMDGPU::SI_SPILL_V320_SAVE;
case 44:
- return AMDGPU::SI_SPILL_V352_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V352_CFI_SAVE
+ : AMDGPU::SI_SPILL_V352_SAVE;
case 48:
- return AMDGPU::SI_SPILL_V384_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V384_CFI_SAVE
+ : AMDGPU::SI_SPILL_V384_SAVE;
case 64:
- return AMDGPU::SI_SPILL_V512_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V512_CFI_SAVE
+ : AMDGPU::SI_SPILL_V512_SAVE;
case 128:
- return AMDGPU::SI_SPILL_V1024_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_V1024_CFI_SAVE
+ : AMDGPU::SI_SPILL_V1024_SAVE;
default:
llvm_unreachable("unknown register size");
}
}
-static unsigned getAVSpillSaveOpcode(unsigned Size) {
+static unsigned getAVSpillSaveOpcode(unsigned Size, bool NeedsCFI) {
switch (Size) {
case 4:
- return AMDGPU::SI_SPILL_AV32_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV32_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV32_SAVE;
case 8:
- return AMDGPU::SI_SPILL_AV64_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV64_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV64_SAVE;
case 12:
- return AMDGPU::SI_SPILL_AV96_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV96_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV96_SAVE;
case 16:
- return AMDGPU::SI_SPILL_AV128_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV128_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV128_SAVE;
case 20:
- return AMDGPU::SI_SPILL_AV160_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV160_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV160_SAVE;
case 24:
- return AMDGPU::SI_SPILL_AV192_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV192_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV192_SAVE;
case 28:
- return AMDGPU::SI_SPILL_AV224_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV224_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV224_SAVE;
case 32:
- return AMDGPU::SI_SPILL_AV256_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV256_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV256_SAVE;
case 36:
return AMDGPU::SI_SPILL_AV288_SAVE;
case 40:
@@ -1629,9 +1654,11 @@ static unsigned getAVSpillSaveOpcode(unsigned Size) {
case 48:
return AMDGPU::SI_SPILL_AV384_SAVE;
case 64:
- return AMDGPU::SI_SPILL_AV512_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV512_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV512_SAVE;
case 128:
- return AMDGPU::SI_SPILL_AV1024_SAVE;
+ return NeedsCFI ? AMDGPU::SI_SPILL_AV1024_CFI_SAVE
+ : AMDGPU::SI_SPILL_AV1024_SAVE;
default:
llvm_unreachable("unknown register size");
}
@@ -1651,7 +1678,7 @@ static unsigned getWWMRegSpillSaveOpcode(unsigned Size,
unsigned SIInstrInfo::getVectorRegSpillSaveOpcode(
Register Reg, const TargetRegisterClass *RC, unsigned Size,
- const SIMachineFunctionInfo &MFI) const {
+ const SIMachineFunctionInfo &MFI, bool NeedsCFI) const {
bool IsVectorSuperClass = RI.isVectorSuperClass(RC);
// Choose the right opcode if spilling a WWM register.
@@ -1660,16 +1687,16 @@ unsigned SIInstrInfo::getVectorRegSpillSaveOpcode(
// TODO: Check if AGPRs are available
if (ST.hasMAIInsts())
- return getAVSpillSaveOpcode(Size);
+ return getAVSpillSaveOpcode(Size, NeedsCFI);
- return getVGPRSpillSaveOpcode(Size);
+ return getVGPRSpillSaveOpcode(Size, NeedsCFI);
}
-void SIInstrInfo::storeRegToStackSlot(
+void SIInstrInfo::storeRegToStackSlotImpl(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, Register SrcReg,
bool isKill, int FrameIndex, const TargetRegisterClass *RC,
- const TargetRegisterInfo *TRI, Register VReg,
- MachineInstr::MIFlag Flags) const {
+ const TargetRegisterInfo *TRI, Register VReg, MachineInstr::MIFlag Flags,
+ bool NeedsCFI) const {
MachineFunction *MF = MBB.getParent();
SIMachineFunctionInfo *MFI = MF->getInfo<SIMachineFunctionInfo>();
MachineFrameInfo &FrameInfo = MF->getFrameInfo();
@@ -1691,7 +1718,8 @@ void SIInstrInfo::storeRegToStackSlot(
// We are only allowed to create one new instruction when spilling
// registers, so we need to use pseudo instruction for spilling SGPRs.
- const MCInstrDesc &OpDesc = get(getSGPRSpillSaveOpcode(SpillSize));
+ const MCInstrDesc &OpDesc =
+ get(getSGPRSpillSaveOpcode(SpillSize, NeedsCFI));
// The SGPR spill/restore instructions only work on number sgprs, so we need
// to make sure we are using the correct register class.
@@ -1710,8 +1738,8 @@ void SIInstrInfo::storeRegToStackSlot(
return;
}
- unsigned Opcode =
- getVectorRegSpillSaveOpcode(VReg ? VReg : SrcReg, RC, SpillSize, *MFI);
+ unsigned Opcode = getVectorRegSpillSaveOpcode(VReg ? VReg : SrcReg, RC,
+ SpillSize, *MFI, NeedsCFI);
MFI->setHasSpilledVGPRs();
BuildMI(MBB, MI, DL, get(Opcode))
@@ -1722,6 +1750,25 @@ void SIInstrInfo::storeRegToStackSlot(
.addMemOperand(MMO);
}
+void SIInstrInfo::storeRegToStackSlot(
+ MachineBasicBlock &MBB, MachineBasicBlock::iterator MI, Register SrcReg,
+ bool isKill, int FrameIndex, const TargetRegisterClass *RC,
+ const TargetRegisterInfo *TRI, Register VReg,
+ MachineInstr::MIFlag Flags) const {
+ storeRegToStackSlotImpl(MBB, MI, SrcReg, isKill, FrameIndex, RC, TRI, VReg,
+ Flags, false);
+}
+
+void SIInstrInfo::storeRegToStackSlotCFI(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MI,
+ Register SrcReg, bool isKill,
+ int FrameIndex,
+ const TargetRegisterClass *RC,
+ const TargetRegisterInfo *TRI) const {
+ storeRegToStackSlotImpl(MBB, MI, SrcReg, isKill, FrameIndex, RC, TRI,
+ Register(), MachineInstr::NoFlags, true);
+}
+
static unsigned getSGPRSpillRestoreOpcode(unsigned Size) {
switch (Size) {
case 4:
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index 5fdeddaf3f736..9c0a80bbcecda 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -293,13 +293,29 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
MachineBasicBlock::iterator I, const DebugLoc &DL,
Register SrcReg, int Value) const;
+private:
+ void storeRegToStackSlotImpl(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MI, Register SrcReg,
+ bool isKill, int FrameIndex,
+ const TargetRegisterClass *RC,
+ const TargetRegisterInfo *TRI, Register VReg,
+ MachineInstr::MIFlag Flags, bool NeedsCFI) const;
+
+public:
+ void storeRegToStackSlotCFI(MachineBasicBlock &MBB,
+ MachineBasicBlock::iterator MI, Register SrcReg,
+ bool isKill, int FrameIndex,
+ const TargetRegisterClass *RC,
+ const TargetRegisterInfo *TRI) const;
+
bool getConstValDefinedInReg(const MachineInstr &MI, cons...
[truncated]
|
arsenm
reviewed
Oct 23, 2025
| return true; | ||
| } | ||
|
|
||
| static bool isLiveIntoMBB(MCRegister Reg, MachineBasicBlock &MBB, |
There was a problem hiding this comment.
This shouldn't be here. I'm still unclear on what the rules are for when register aliases will appear in the live in list. This also should account for the LaneMask
| ; GCN-NEXT: buffer_store_dword v40, off, s[0:3], s33 ; 4-byte Folded Spill | ||
| ; GCN-NEXT: s_mov_b64 exec, s[6:7] | ||
| ; GCN-NEXT: v_writelane_b32 v40, s4, 2 | ||
| ; GCN-NEXT: v_writelane_b32 v40, s30, 0 |
There was a problem hiding this comment.
I'm not sure why this results in all of these scheduling changes
There was a problem hiding this comment.
There are a few things leading to the scheduling changes, and I will split up this change into multiple to tease them apart so we can consider them independently.
The biggest issue is that the postmisched which runs after prologepilog (conservatively) sees the CFI_INSTRUCTIONs as scheduling boundaries, so e.g. the sink of v_readlane_b32 s30, v40, 0 from its original spot in the prolog past the CFI_INSTRUCTION which describes it is not valid.
This is why we force the CFI emission in all cases for AMDGPU, because we really want to avoid spurious scheduling changes which only occur when debug-info is enabled. If enabling debug-info can actually change program behavior then it isn't terribly useful. I do think we should move to just making the default be CFI-enabled, and still respect nounwind for e.g. test cases.
I dug up https://bugs.llvm.org/show_bug.cgi?id=37240 and https://reviews.llvm.org/D71350 concerning the larger problem, and AFAIK there isn't a great general solution yet. X86 seems to dodge the issue by not enabling a post-RA scheduler by default. arm/aarch64 just seem to have the bug currently.
I spent a couple days trying to sketch out what a better fix might look like, but it will be a large effort, and I would rather get the changes we have already been relying on for unwind info upstream first, if that is acceptable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce new SPILL pseudos to allow CFI to be generated for only CSR
spills, and to make ISA-instruction-level accurate information.
Other targets either generate slightly incorrect information or rely on
conventions for how spills are placed within the entry block. The
approach in this change produces larger unwind tables, with the
increased size being spent on additional DW_CFA_advance_location
instructions needed to describe the unwinding accurately.
Co-authored-by: Scott Linder scott.linder@amd.com
Co-authored-by: Venkata Ramanaiah Nalamothu VenkataRamanaiah.Nalamothu@amd.com