[amdgpu-cfi: 5/9]: [AMDGPU] Implement CFI for non-kernel functions - #183153
Merged
slinder1 merged 5 commits intoMay 26, 2026
Merged
Conversation
slinder1
changed the base branch from
main
to
users/slinder1/I21580f6a24f4869ba32939c9c6332506032cc654
February 24, 2026 19:57
slinder1
changed the title
This was referenced Feb 24, 2026
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
slinder1
force-pushed
the
users/slinder1/I21580f6a24f4869ba32939c9c6332506032cc654
branch
from
February 24, 2026 21:41
97a81b4 to
a77b5c2
Compare
slinder1
force-pushed
the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
from
February 24, 2026 21:41
e4f5784 to
9dc067f
Compare
slinder1
changed the title
slinder1
force-pushed
the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
from
February 24, 2026 22:13
9dc067f to
36fa2c3
Compare
Contributor
Author
|
Changes since last push: diff --git b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -329,14 +329,14 @@ class PrologEpilogSGPRSpillBuilder {
/// SubReg spill, as each SubReg maps directly to a CFI register via
/// getDwarfRegNum(SubReg, false). All other cases currently currently
/// correspond to the SuperReg directly.
- std::optional<Register> getCFISuperReg() const {
+ MCRegister getCFISuperReg() const {
if (IsFramePtrPrologSpill)
return FuncInfo->getFrameOffsetReg();
// FIXME: CFI for EXEC needs a fix by accurately computing the spill
// offset for both the low and high components.
if (isExec(SuperReg))
return AMDGPU::EXEC;
- return std::nullopt;
+ return {};
}
void saveToMemory(const int FI) const {
@@ -357,7 +357,7 @@ class PrologEpilogSGPRSpillBuilder {
nullptr, MCRI->getDwarfRegNum(Reg, false),
MFI.getObjectOffset(FI) * ST.getWavefrontSize()));
};
- std::optional<Register> CFISuperReg = getCFISuperReg();
+ MCRegister CFISuperReg = getCFISuperReg();
for (unsigned I = 0, DwordOff = 0; I < NumSubRegs; ++I) {
Register SubReg = NumSubRegs == 1
? SuperReg
@@ -372,7 +372,7 @@ class PrologEpilogSGPRSpillBuilder {
DwordOff += 4;
}
if (NeedsFrameMoves && CFISuperReg)
- BuildCFI(*CFISuperReg);
+ BuildCFI(CFISuperReg);
}
void saveToVGPRLane(const int FI) const {
@@ -383,7 +383,7 @@ class PrologEpilogSGPRSpillBuilder {
FuncInfo->getSGPRSpillToPhysicalVGPRLanes(FI);
assert(Spill.size() == NumSubRegs);
- std::optional<Register> CFISuperReg = getCFISuperReg();
+ MCRegister CFISuperReg = getCFISuperReg();
for (unsigned I = 0; I < NumSubRegs; ++I) {
Register SubReg = NumSubRegs == 1
? SuperReg
@@ -394,11 +394,11 @@ class PrologEpilogSGPRSpillBuilder {
.addImm(Spill[I].Lane)
.addReg(Spill[I].VGPR, RegState::Undef);
if (NeedsFrameMoves && !CFISuperReg)
- TFI->buildCFIForSGPRToVGPRSpill(MBB, MI, DL, SubReg, Spill[I].VGPR,
- Spill[I].Lane);
+ TFI->buildCFIForSGPRToVGPRSpill(MBB, MI, DL, SubReg, Spill[I].VGPR,
+ Spill[I].Lane);
}
if (NeedsFrameMoves && CFISuperReg)
- TFI->buildCFIForSGPRToVGPRSpill(MBB, MI, DL, *CFISuperReg, Spill);
+ TFI->buildCFIForSGPRToVGPRSpill(MBB, MI, DL, CFISuperReg, Spill);
}
void copyToScratchSGPR(Register DstReg) const {
@@ -411,12 +411,12 @@ class PrologEpilogSGPRSpillBuilder {
unsigned DstNumSubRegs = DstSplitParts.empty() ? 1 : DstSplitParts.size();
assert(NumSubRegs == DstNumSubRegs);
for (unsigned I = 0; I < NumSubRegs; ++I) {
- Register SrcSubReg =
- NumSubRegs == 1 ? SuperReg
- : Register(TRI.getSubReg(SuperReg, SplitParts[I]));
- Register DstSubReg =
- NumSubRegs == 1 ? DstReg
- : Register(TRI.getSubReg(DstReg, DstSplitParts[I]));
+ MCRegister SrcSubReg = NumSubRegs == 1
+ ? MCRegister(SuperReg)
+ : TRI.getSubReg(SuperReg, SplitParts[I]);
+ MCRegister DstSubReg = NumSubRegs == 1
+ ? MCRegister(DstReg)
+ : TRI.getSubReg(DstReg, DstSplitParts[I]);
if (isExec(SuperReg)) {
if (I == NumSubRegs - 1)
TFI->buildCFIForRegToSGPRPairSpill(MBB, MI, DL, AMDGPU::EXEC,
@@ -442,9 +442,9 @@ class PrologEpilogSGPRSpillBuilder {
report_fatal_error("failed to find free scratch register");
for (unsigned I = 0, DwordOff = 0; I < NumSubRegs; ++I) {
- Register SubReg = NumSubRegs == 1
- ? SuperReg
- : Register(TRI.getSubReg(SuperReg, SplitParts[I]));
+ MCRegister SubReg = NumSubRegs == 1
+ ? MCRegister(SuperReg)
+ : TRI.getSubReg(SuperReg, SplitParts[I]);
buildEpilogRestore(ST, TRI, *FuncInfo, LiveUnits, MF, MBB, MI, DL,
TmpVGPR, FI, FrameReg, DwordOff);
@@ -463,9 +463,9 @@ class PrologEpilogSGPRSpillBuilder {
assert(Spill.size() == NumSubRegs);
for (unsigned I = 0; I < NumSubRegs; ++I) {
- Register SubReg = NumSubRegs == 1
- ? SuperReg
- : Register(TRI.getSubReg(SuperReg, SplitParts[I]));
+ MCRegister SubReg = NumSubRegs == 1
+ ? MCRegister(SuperReg)
+ : TRI.getSubReg(SuperReg, SplitParts[I]);
BuildMI(MBB, MI, DL, TII->get(AMDGPU::SI_RESTORE_S32_FROM_VGPR), SubReg)
.addReg(Spill[I].VGPR)
.addImm(Spill[I].Lane);
@@ -2517,16 +2517,15 @@ MachineInstr *SIFrameLowering::buildCFIForRegToSGPRPairSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
const DebugLoc &DL, const Register Reg, const Register SGPRPair) const {
const MachineFunction &MF = *MBB.getParent();
- const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
const SIRegisterInfo &TRI = ST.getInstrInfo()->getRegisterInfo();
MCRegister SGPR0 = TRI.getSubReg(SGPRPair, AMDGPU::sub0);
MCRegister SGPR1 = TRI.getSubReg(SGPRPair, AMDGPU::sub1);
- int DwarfReg = MCRI.getDwarfRegNum(Reg, false);
- int DwarfSGPR0 = MCRI.getDwarfRegNum(SGPR0, false);
- int DwarfSGPR1 = MCRI.getDwarfRegNum(SGPR1, false);
+ int DwarfReg = TRI.getDwarfRegNum(Reg, false);
+ int DwarfSGPR0 = TRI.getDwarfRegNum(SGPR0, false);
+ int DwarfSGPR1 = TRI.getDwarfRegNum(SGPR1, false);
assert(DwarfReg != -1 && DwarfSGPR0 != 1 && DwarfSGPR1 != 1);
auto CFIInst = MCCFIInstruction::createLLVMRegisterPair(
|
2 tasks
slinder1
force-pushed
the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
from
February 25, 2026 20:58
36fa2c3 to
e25ca09
Compare
slinder1
force-pushed
the
users/slinder1/I21580f6a24f4869ba32939c9c6332506032cc654
branch
from
February 25, 2026 20:58
a77b5c2 to
2e8751f
Compare
slinder1
changed the title
slinder1
force-pushed
the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
from
February 26, 2026 21:21
e25ca09 to
aeeedc7
Compare
Contributor
Author
|
Changes since last push: diff --git b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -96,7 +96,7 @@ createScaledCFAInPrivateWave(const GCNSubtarget &ST,
void SIFrameLowering::emitDefCFA(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
- DebugLoc const &DL, Register StackPtrReg,
+ DebugLoc const &DL, MCRegister StackPtrReg,
bool AspaceAlreadyDefined,
MachineInstr::MIFlag Flags) const {
MachineFunction &MF = *MBB.getParent();
@@ -1112,7 +1112,7 @@ void SIFrameLowering::emitPrologueEntryCFI(MachineBasicBlock &MBB,
const MCRegisterInfo *MCRI = MF.getContext().getRegisterInfo();
const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
const SIRegisterInfo &TRI = ST.getInstrInfo()->getRegisterInfo();
- Register StackPtrReg =
+ MCRegister StackPtrReg =
MF.getInfo<SIMachineFunctionInfo>()->getStackPtrOffsetReg();
emitDefCFA(MBB, MBBI, DL, StackPtrReg, /*AspaceAlreadyDefined=*/true,
@@ -2464,7 +2464,7 @@ MachineInstr *SIFrameLowering::buildCFI(MachineBasicBlock &MBB,
MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
- const DebugLoc &DL, const Register SGPR, const Register VGPR,
+ const DebugLoc &DL, const MCRegister SGPR, const MCRegister VGPR,
const int Lane) const {
const MachineFunction &MF = *MBB.getParent();
const MCRegisterInfo &MCRI = *MF.getContext().getRegisterInfo();
@@ -2485,7 +2485,7 @@ MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
- const DebugLoc &DL, Register SGPR,
+ const DebugLoc &DL, MCRegister SGPR,
ArrayRef<SIRegisterInfo::SpilledReg> VGPRSpills) const {
if (VGPRSpills.size() == 1u)
return buildCFIForSGPRToVGPRSpill(MBB, MBBI, DL, SGPR, VGPRSpills[0].VGPR,
@@ -2515,7 +2515,7 @@ MachineInstr *SIFrameLowering::buildCFIForSGPRToVGPRSpill(
MachineInstr *SIFrameLowering::buildCFIForRegToSGPRPairSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
- const DebugLoc &DL, const Register Reg, const Register SGPRPair) const {
+ const DebugLoc &DL, const MCRegister Reg, const MCRegister SGPRPair) const {
const MachineFunction &MF = *MBB.getParent();
const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
const SIRegisterInfo &TRI = ST.getInstrInfo()->getRegisterInfo();
diff --git b/llvm/lib/Target/AMDGPU/SIFrameLowering.h a/llvm/lib/Target/AMDGPU/SIFrameLowering.h
@@ -106,7 +106,7 @@ private:
const DebugLoc &DL) const;
void emitDefCFA(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
- DebugLoc const &DL, Register StackPtrReg,
+ DebugLoc const &DL, MCRegister StackPtrReg,
bool AspaceAlreadyDefined,
MachineInstr::MIFlag Flags = MachineInstr::NoFlags) const;
@@ -124,19 +124,20 @@ public:
MachineInstr *buildCFIForSGPRToVGPRSpill(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
const DebugLoc &DL,
- const Register SGPR,
- const Register VGPR,
+ const MCRegister SGPR,
+ const MCRegister VGPR,
const int Lane) const;
/// Create a CFI index describing a spill of an SGPR to multiple lanes of
/// VGPRs and build a MachineInstr around it.
MachineInstr *buildCFIForSGPRToVGPRSpill(
MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
- const DebugLoc &DL, Register SGPR,
+ const DebugLoc &DL, MCRegister SGPR,
ArrayRef<SIRegisterInfo::SpilledReg> VGPRSpills) const;
MachineInstr *buildCFIForRegToSGPRPairSpill(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI,
- const DebugLoc &DL, Register Reg,
- Register SGPRPair) const;
+ const DebugLoc &DL,
+ MCRegister Reg,
+ MCRegister SGPRPair) 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;
|
slinder1
requested review from
OCHyams,
SLTozer,
adrian-prantl,
arsenm,
epilk,
jmorse and
kzhuravl
February 26, 2026 21:21
Base automatically changed from
users/slinder1/I21580f6a24f4869ba32939c9c6332506032cc654
to
main
May 26, 2026 20:07
Previously we effectively took the absolute value of the APSInt, instead diagnose the unexpected negative value. Change-Id: I4efe961e7b29fdf1d5f97df12f8139aac12c9219
While these can be represented with .cfi_escape, using these pseudo-cfi instructions makes .s/.mir files more readable, and it is necessary to support updating registers in CFI instructions (something that the AMDGPU backend requires). Change-Id: I763d0cabe5990394670281d4afb5a170981e55d0
To avoid codegen changes when enabling debug-info (see https://bugs.llvm.org/show_bug.cgi?id=37240) we want to enable unwind tables by default. There is some pessimization in post-prologepilog scheduling, and a general solution to the problem of CFI_INSTRUCTION-as-scheduling-barrier should be explored. Change-Id: I83625875966928c7c4411cd7b95174dc58bda25a
Entry functions represent the end of unwinding, as they are the outer-most frame. This implies they can only have a meaningful definition for the CFA, which AMDGPU defines using a memory location description with a literal private address space address. The return address is set to undefined as a sentinel value to signal the end of unwinding. Change-Id: I21580f6a24f4869ba32939c9c6332506032cc654 Co-authored-by: Scott Linder <scott.linder@amd.com> Co-authored-by: Venkata Ramanaiah Nalamothu <VenkataRamanaiah.Nalamothu@amd.com>
This does not implement CSR spills other than those AMDGPU handles during PEI. The remaining spills are handled in a subsequent patch. Change-Id: I5e3a9a62cf9189245011a82a129790d813d49373 Co-authored-by: Scott Linder <scott.linder@amd.com> Co-authored-by: Venkata Ramanaiah Nalamothu <VenkataRamanaiah.Nalamothu@amd.com>
slinder1
force-pushed
the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
from
May 26, 2026 20:08
0fd81b2 to
bd8d788
Compare
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
slinder1
deleted the
users/slinder1/I5e3a9a62cf9189245011a82a129790d813d49373
branch
May 26, 2026 20:57
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/228/builds/105 Here is the relevant piece of the build log for the reference |
slinder1
added a commit
that referenced
this pull request
May 26, 2026
Attempt at fixing issues in #183153 caught by buildbots, specifically no-assert and windows builds. Not sure how to run those bots ahead of landing this? Change-Id: I285adf09ac2df239d0ab05459f7388b6970247ad
schuermans-roofline
added a commit
to iree-org/llvm-project
that referenced
this pull request
May 27, 2026
This is a fix for "[AMDGPU] Implement CFI for non-kernel functions (llvm#183153)" f78a233 to use "SuperReg.asMCReg()" instead of "MCRegister(SuperReg)", which leads to "ambiguous call" when using the MSVC compiler.
| if (NeedsFrameMoves) { | ||
| const TargetRegisterClass *RC = TRI.getPhysRegBaseClass(DstReg); | ||
| ArrayRef<int16_t> DstSplitParts = TRI.getRegSplitParts(RC, EltSize); | ||
| unsigned DstNumSubRegs = DstSplitParts.empty() ? 1 : DstSplitParts.size(); |
Contributor
There was a problem hiding this comment.
Hi @slinder1
DstNumSubRegs is unused in builds without asserts:
/repo/llvm/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:412:16: error: unused variable 'DstNumSubRegs' [-Werror,-Wunused-variable]
412 | unsigned DstNumSubRegs = DstSplitParts.empty() ? 1 : DstSplitParts.size();
| ^~~~~~~~~~~~~
1 error generated.
RKSimon
reviewed
May 27, 2026
| ? SuperReg | ||
| : Register(TRI.getSubReg(SuperReg, SplitParts[I])); | ||
| MCRegister SubReg = NumSubRegs == 1 | ||
| ? MCRegister(SuperReg) |
Contributor
There was a problem hiding this comment.
@slinder1
MSVC is complaining:
error C2440: '<function-style-cast>': cannot convert from 'llvm::Register' to 'llvm::MCRegister'
Contributor
Author
There was a problem hiding this comment.
I'm working on reproducing locally (have to set up a windows build, recently got a new laptop) but if there is any way to test #199781 against a buildbot I think it might fix this
| ? SuperReg | ||
| : Register(TRI.getSubReg(SuperReg, SplitParts[I])); | ||
| MCRegister SubReg = NumSubRegs == 1 | ||
| ? MCRegister(SuperReg) |
Contributor
There was a problem hiding this comment.
@slinder1
MSVC is complaining:
error C2440: '<function-style-cast>': cannot convert from 'llvm::Register' to 'llvm::MCRegister'
schuermans-roofline
added a commit
to schuermans-roofline/llvm-project
that referenced
this pull request
May 27, 2026
This change adapts the bazel build system for the change in: [BBAddrMap] Drive Features and Metadata bits from BBAddrMap.def (llvm#196906)pick 7ea8e6eb3ee7 Revert "[AMDGPU] Implement CFI for non-kernel functions (llvm#183153)" commit 532940b The commit had added a .def file, which was not found by the glob for .h in the bazel build file. This change adds a glob for *.def to fix this.
arsenm
pushed a commit
that referenced
this pull request
May 27, 2026
slinder1
added a commit
that referenced
this pull request
May 27, 2026
Change-Id: I285adf09ac2df239d0ab05459f7388b6970247ad
slinder1
added a commit
that referenced
this pull request
May 27, 2026
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
1 similar comment
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
ronlieb
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
May 27, 2026
This is a fix for "[AMDGPU] Implement CFI for non-kernel functions (llvm#183153)" f78a233 to use "SuperReg.asMCReg()" instead of "MCRegister(SuperReg)", which leads to "ambiguous call" when using the MSVC compiler.
ronlieb
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
May 27, 2026
Change-Id: I285adf09ac2df239d0ab05459f7388b6970247ad
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
1 similar comment
Contributor
Author
|
🛠️ Changes since last push: none (likely a rebase) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
This does not implement CSR spills other than those AMDGPU handles
during PEI. The remaining spills are handled in a subsequent patch.
Change-Id: I5e3a9a62cf9189245011a82a129790d813d49373
Co-authored-by: Scott Linder scott.linder@amd.com
Co-authored-by: Venkata Ramanaiah Nalamothu VenkataRamanaiah.Nalamothu@amd.com
Stack:
main(Note: Closed and merged PRs may not be reflected here and PR numbering is not stable.)