[PATCH] D78811: [AMDGPU] Enable base pointer.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:756
+  if (FuncInfo->BasePointerSaveIndex) {
+    const int BasePtrFI = *(FuncInfo->BasePointerSaveIndex);
+    assert(!MFI.isDeadObjectIndex(BasePtrFI) &&
----------------
Don't need the parentheses


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:118
+  // stack pointer, so we reserve a base pointer.
+  return needsStackRealignment(MF);
+}
----------------
cdevadas wrote:
> arsenm wrote:
> > cdevadas wrote:
> > > scott.linder wrote:
> > > > Is it possible to have some mechanism for only setting up the BP when it is actually used? Looking at X86 it seems like they don't do this, so I am OK with us not doing it either, just wondering if it is feasible.
> > > It sounds good to me. We can enable it only when there is a fixed-frame. 
> > > It can be easily done with an additional check, MFI.getNumFixedObjects() != 0
> > I think getNumFixedObjects is not the right check here. That won't deal with the case where we have dead stack objects (as is common from eliminated SGPR spils)
> It would be good to have a straight forward check here to see if there is any fixed-frame and getNumFixedObjects would do that.
> 
>  
It doesn't, because for us it's full of litter from SGPR spills. This will be overly conservative, and trigger when we have no actual stack objects. Is this only called after the frame is finalized?

The way the x86 version expresses this makes more sense to me:
ister.
   bool CantUseFP = needsStackRealignment(MF);
   return CantUseFP && CantUseSP(MFI);

This might work for us the same?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78811