Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[AMDGPU] Add amdgpu_cs_chain[_preserve] CCs to IR & verifier

Authored by rovka on Jun 2 2023, 6:39 AM.



Add the amdgpu_cs_chain and amdgpu_cs_chain_preserve keywords to
LLVM IR and make sure we can parse and print them. Also make sure we
perform some basic checks in the IR verifier - similar to what we check
for many of the other AMDGPU calling conventions, plus the additional
restriction that we can't have direct calls to functions with these
calling conventions.

Diff Detail

Event Timeline

rovka created this revision.Jun 2 2023, 6:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2023, 6:39 AM
rovka requested review of this revision.Jun 2 2023, 6:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2023, 6:39 AM
Herald added a subscriber: wdng. · View Herald Transcript
foad added a subscriber: foad.Jun 6 2023, 1:39 AM

Please upload the patch with full context.


Should check Call.getCallingConv(), instead of relying on whether or not we found a Callee function.

arsenm added a subscriber: arsenm.Jun 6 2023, 5:37 AM
arsenm added inline comments.

Is a regular pointer to the stack allowed?

nhaehnle added inline comments.Jun 6 2023, 11:25 AM

You mean a ptr addrspace(5)? Yes, it's allowed, and we plan to use it.

It's technically not going to be a *stack* pointer as far as LLVM is concerned, because the stack is "dormant"/"inexistent" between chain functions. The plan is to split the wave's scratch allocation into two parts, the bottom part under the control of LLVM and the top part under the control of the frontend, and so passing pointers to that top part can still be meaningful even though the stack "doesn't exist" at chain function boundaries as far as LLVM is concerned.

In principle, we also allow pointers to other address spaces, although I don't see us using those at least for the time being.

rovka updated this revision to Diff 529248.Jun 7 2023, 4:35 AM

Address review comments.

arsenm added inline comments.Jun 8 2023, 4:42 PM

Should say how the _Preserve variant differs

rovka updated this revision to Diff 529921.Jun 9 2023, 5:12 AM

Mention what amdgpu_cs_chain_preserve preserves.

Any other comments or is this good to go?

nhaehnle accepted this revision.Jun 21 2023, 2:46 AM
This revision is now accepted and ready to land.Jun 21 2023, 2:46 AM
arsenm added inline comments.Jun 21 2023, 3:46 AM

Check a call to a function pointer

rovka marked an inline comment as done.Jun 22 2023, 1:07 AM