[PATCH] D124615: [AArch64][SVE] Restore SP from base pointer when it and SVE frame are present

Fri Apr 29 05:10:16 PDT 2022

paulwalker-arm added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1961
+      TII->copyPhysReg(MBB, RestoreBegin, DL, AArch64::SP,
+                       RegInfo.getBaseRegister(), false);
+
----------------
bsmith wrote:
> efriedma wrote:
> > Do we need to use emitFrameOffset() here?
> I'm not sure we should/need to, the allocation of the variable length array that leads to the changing of the stack pointer does not happen during frame lowering, but rather during isel lowering, hence it does not have the usual frame-setup flag etc. As such I'm not sure this, which is just reverting that, should have the frame-destroy flag etc either, however I'm not sure about this.
Should we be doing this when `AFI->isStackRealigned()` is true?  It looks like for that case we recalculate SP from the frame pointer so this'll kick out a redundant instruction. I guess there's no test to show the negative affect of this, it's just something I spotted whilst working on another bug.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124615