This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][PEI] Set up SP for chain functions
ClosedPublic

Authored by rovka on Jul 27 2023, 3:14 AM.

Details

Reviewers
arsenm
Group Reviewers
Restricted Project
Commits
rG39830fea284f: [AMDGPU][PEI] Set up SP for chain functions
Summary

Initialize the SP to 0 in the prologue of functions with the
amdgpu_cs_chain or amdgpu_cs_chain_preserve calling conventions, but
only if they need one (i.e. if they contain calls to amdgpu_gfx
functions or if they have stack objects).

Diff Detail

Unit TestsFailed

Event Timeline

rovka created this revision.Jul 27 2023, 3:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 3:14 AM
rovka requested review of this revision.Jul 27 2023, 3:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 3:14 AM
rovka added a reviewer: Restricted Project.

Can you add a test where this would need stack realignment? You should be able to bypass all of that stuff (I think we have a calling convention override for that already, you might set that somewhere else)

llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1094

We should probably get rid of SP_REG, it's always just hardcoded anyway

rovka updated this revision to Diff 545550.Jul 31 2023, 2:24 AM

Make sure we don't try to realign the stack.

arsenm added inline comments.Aug 11 2023, 8:43 AM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1092

Should use requiresStackPointerReference?

arsenm requested changes to this revision.Aug 14 2023, 2:21 PM

Should try to avoid repeating the SP reference logic

This revision now requires changes to proceed.Aug 14 2023, 2:21 PM
rovka updated this revision to Diff 551082.Aug 17 2023, 3:58 AM

Use requiresStackPointerReference. We still need to check hasStackObjects though, otherwise we end up using an uninitialized s32 for spills and allocas. Ideally we'd be able to use an immediate for those cases, but I haven't figured out how to do that yet, so I left a FIXME.

arsenm accepted this revision.Aug 17 2023, 6:31 AM
This revision is now accepted and ready to land.Aug 17 2023, 6:31 AM
This revision was automatically updated to reflect the committed changes.