[PATCH] D75138: [WIP][AMDGPU] Add Scratch Wave Offset to Scratch Buffer Descriptor in entry functions

Wed Mar 11 11:19:18 PDT 2020

scott.linder added a comment.

In D75138#1916158 <https://reviews.llvm.org/D75138#1916158>, @t-tye wrote:

> I think commit comment "The ABI stack pointer register remains unswizzled, but is now wave-relative instead of dispatch-relative." shuld chage to "The ABI stack pointer register remains unswizzled, but is now wave-relative instead of queue-relative." since for the HSAABI the scratch base is the queue base and not per dispatch. The PALABI may use per dispatch scratch allocation.

I updated the commit message, but I didn't include mention of the possibility of the PALABI differing here. Is there a more generic way to describe the old behavior for every ABI the compiler supports? As far as the compiler is concerned it is only important that the SRSRC base + the scratch wave offset gets it to the base for the scratch allocation for the wave.

================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:290
+  unsigned NumPreloadedSGPRs = MFI->getNumPreloadedSGPRs();
+  // FIXME: This is just lifted from AMDGPUAsmPrinter, because I'm not
+  // sure where/if we track InReg SGPR arguments otherwise.
----------------
arsenm wrote:
> scott.linder wrote:
> > arsenm wrote:
> > > This should not need to inspect the original IR. Why can't this just read it directly from MFI? They should be accounted there already?
> > @arsenm @nhaehnle I don't think I understand how `inreg` currently works relative to "preloaded" SGPRs; is/should `inreg` be recorded somewhere in the machine function info so this isn't necessary?
> Not directly. There shouldn't be any repeating of the calling convention logic here. Either the number of SGPR arguments should be recorded, or it should be inferred from the machine code.
> 
> It might be correct to just count the number of SGPR in the function live-in list. I think live in registers can be deleted from the list if they are proven to be unused, so this might be fragile. Finding the highest live in SGPR number may also work.
In switching back to supporting an FP I no longer see the need for this manifest, but there may still be a need to update this in the future. I don't think my change is making this any more fragile so I'm leaving it as it was.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75138