[PATCH] D70174: [AArch64][SVE] Use FP for scavenging slot

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp:327
   const AArch64FrameLowering &TFI = *getFrameLowering(MF);
-  return TFI.hasFP(MF);
+  return TFI.hasFP(MF) && !TFI.estimateSVEStackObjectOffsets(MF.getFrameInfo());
 }
----------------
sdesmalen wrote:
> efriedma wrote:
> > The problem here, I guess, is the spill slot can't be addressed relative to SP? That makes sense, I guess... but do we also need to check for stack realignment here?
> Apologies for my delayed repsonse, I completely forgot about this patch.
> 
> > The problem here, I guess, is the spill slot can't be addressed relative to SP?
> In that case it will be allocated close to the BP. If there are variable-sized objects the choices are either BP or FP, otherwise the choices are SP or FP. Because the emergency spillslot needs to be accessed using the normal (non-SVE) addressing modes in one go (i.e. without requiring another register to materialize the address to the emergency slot), it needs to be able to access it directly from Pointer + <offset>, so BP or SP are really the only options. Otherwise it would need a temporary register to calculate `FP - sizeof(SVE area)`, which itself would require the emergency slot to materialise the temporary.
> 
> > do we also need to check for stack realignment here?
> I don't think that's specifically needed, because `hasFP()` will always evaluate to `true` if the stack requires realignment. Do you want me to add an `assert` for that?
I think you're misunderstanding what I'm saying about stack realignment.  I think the interaction with stack realignment is actually broken (and nobody has run into it because stack realignment is rare on AArch64).  I'm thinking that the condition here should just be `TFI.hasFP(MF) && !hasBasePointer(MF)`.


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

https://reviews.llvm.org/D70174