[PATCH] D81364: AMDGPU Initializes StackPtrOffset when NonSpillStackObjects present

[David Stuttard via Phabricator via llvm-commits]

dstuttard marked an inline comment as done.
dstuttard added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:463
+  // initialize SP in case it is required.
+  if (MF.getFrameInfo().hasCalls() || MFI->hasNonSpillStackObjects()) {
     Register SPReg = MFI->getStackPtrOffsetReg();
----------------
arsenm wrote:
> arsenm wrote:
> > dstuttard wrote:
> > > Is there a way to finesse this so it's only added when it is likely to be required?
> > > 
> > This isn't really right either. hasNonSpillStackObjects isn't accurate, since we can access with an soffset of 0 in the entry point. This also will fail to catch cases with variable allocas. I think the logic here should probably be hasFP()?
> I think this really is hasFP based on the implementation of getFrameRegister
I tried hasFP and it didn't resolve the original issue I was fixing. I'm not 100% sure what's going on here now. The backend definitely generates an access using stackPtrOffsetReg when nonSpillStackObjects are present. Should the test include both?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81364