[PATCH] D81364: AMDGPU Initializes StackPtrOffset when NonSpillStackObjects present

[Matt Arsenault via Phabricator via llvm-commits]

arsenm 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();
----------------
dstuttard wrote:
> 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?
Do you have the testcase for the original issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81364