[PATCH] D153517: [AMDGPU] ISel for amdgpu_cs_chain[_preserve] functions

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:91
+    // right after the arguments.
+    StackPtrOffsetReg = AMDGPU::SGPR105;
+
----------------
rovka wrote:
> arsenm wrote:
> > This is an odd choice of value and it doesn't exist on all subtargets
> I tried setting the SP in `finalizeLowering` using basically [[ https://github.com/llvm/llvm-project/blob/5cd16e2cb7207a70a61d1221464b17dcaf3c7f29/llvm/lib/Target/AMDGPU/SIISelLowering.cpp#L2380 | this snippet ]], but it blew up for Global ISel (I think because of some issue with the liveins). I figured I should ask here before going too far down that rabbit hole - does that sound like the right approach, or is there a better place to handle the SP? I'm guessing there are other things I could prioritize before fixing this, but do let me know if you think otherwise.
Realistically, we're only ever going to use this on gfx10+, where the register always exists. If you're too worried, we could have a corresponding assertion.

That said, it's not clear to me what this value can end up being used for, since chain functions don't have a stack pointer input or output. A stack pointer should be set up if we call an amdgpu_gfx function; does it matter whether the chosen register is aligned with that calling convention?


================
Comment at: llvm/test/CodeGen/AMDGPU/amdgpu-cs-chain-preserve-cc.ll:16
+
+define amdgpu_cs_chain_preserve void @amdgpu_cs_chain_preserve_cc(<4 x i32> inreg %a, <4 x i32> %b) {
+  ; GISEL-GFX11-LABEL: name: amdgpu_cs_chain_preserve_cc
----------------
These functions probably need way more scratch save/restore. Actually, I'm starting to suspect that we can't reliably code-generate amdgpu_cs_chain_preserve functions that call amdgpu_gfx functions, because the amdgpu_gfx calling convention has some caller-saves registers, which just doesn't mesh well.

This probably needs a bit more thought, though it shouldn't be a big problem in practice: the purpose / intended use of the _preserve calling convention is very narrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153517/new/

https://reviews.llvm.org/D153517