[PATCH] D40876: AArch64: Fix emergency spillslot being out of reach for large callframes

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:213
+  // DefaultSafeSPDisplacement is fine as we only emergency spill GP regs.
+  if (!MFI.isMaxCallFrameSizeComputed() ||
+      MFI.getMaxCallFrameSize() > DefaultSafeSPDisplacement)
----------------
efriedma wrote:
> gberry wrote:
> > efriedma wrote:
> > > isMaxCallFrameSizeComputed() is false until after register allocation, right?  That means register allocation will never allocate a value into the frame pointer register.  This needs better comments, and I'm not sure we want to unconditionally reserve the frame pointer register just to fix an obscure bug with very large call frames.
> > Part of this change is to call computeMaxCallFrameSize() from finalizeLowering() (called at the end of ISel), so I don't think this is a problem?
> Oh, sorry, missed that part.  That's much less scary, then.  The only thing I can think of it might impact is inline asm.  Do we have any testcases with inline asm using the frame pointer as an operand or result?
I also added an early `computeMaxCallFrameSize()` to the ARM target a while ago and so far it is working fine.

As for inline assembly:
- Note that this only changes behavior in code with very unusual calls having a huge number of parameters.
- I would consider omitting the frame pointer an optimization that you cannot/should not rely on as there are various factors outside the authors control that could influence whether the compiler is able to perform it.
- There is no inline asm constraint letter defined for FP as far as I can see (so you only could rely on it indirectly by having so many register arguments to inline assembly that you need to use the FP).


https://reviews.llvm.org/D40876