This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Handle llvm.stacksave and llvm.stackrestore
ClosedPublic

Authored by arsenm on Jul 31 2023, 4:24 AM.

Details

Reviewers
Pierre-vh
foad
rampitec
JonChesterfield
doru1004
Group Reviewers
Restricted Project
Summary

Not sure if the only valid use is to have stackrestore directly
consume stacksave outputs or not. Handled exactly like a regular stack
pointer so all the edge cases theoretically should work.

Diff Detail

Event Timeline

arsenm created this revision.Jul 31 2023, 4:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 4:24 AM
arsenm requested review of this revision.Jul 31 2023, 4:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 4:24 AM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm updated this revision to Diff 545593.Jul 31 2023, 4:42 AM

Add documentation (really need to replace this insufferable table with the less whitespace dependent syntax form)

Pierre-vh added inline comments.Aug 4 2023, 6:17 AM
llvm/docs/ReleaseNotes.rst
77

Should this mention it's only for p5?

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
2582

So this does:

swizzled SP = unswizzled SP / wavefront size

But the docs say it does the opposite? (converts a swizzled address to unswizzled?)

My understanding of swizzling is pretty shallow, a bit more docs to help clarify this would be nice

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3537

How does changing the SP address to the swizzled SP help with edge cases?
Also why do we need a new opcode for this?

arsenm added inline comments.Aug 9 2023, 2:51 PM
llvm/docs/ReleaseNotes.rst
77

Not really, it's all that makes sense. the AMDGPUUsage note is enough

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp
2582

I think the terminology is "swizzled" = "vgpr offset" and "unswizzled" = "sgpr offset" to a MUBUF instruction

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3537

An IR pointer is a vector value. A raw read of SP is the scalar value. The stack increments are really in wavesize chunks. If you do any addressing, or access of the pointer using the MUBUF voffset field you'll end up in the wrong place. The real usage is just write the direct value of a stacksave to a stackrestore call.

We need a new opcode to preserve the semantic intent. Otherwise the fold (whether into an addressing mode, or into the stackrestore call) will need to guess at the interpretation for a copy from a physical register. The call lowering has the same problem for matching the MUBUF addressing modes (GlobalISel uses the pseudo, the DAG gets away with inspecting the PseudoSourceValue for the underlying pointer)

arsenm updated this revision to Diff 548780.Aug 9 2023, 3:07 PM

Fix comment

Pierre-vh accepted this revision.Aug 11 2023, 6:07 AM
This revision is now accepted and ready to land.Aug 11 2023, 6:07 AM