[PATCH] D71168: [AArch64] Save FP for leaf functions when disabling frame pointer elimination

Fri Dec 13 15:18:29 PST 2019

MaskRay marked an inline comment as done.
MaskRay added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:247
   // Retain behavior of always omitting the FP for leaf functions when possible.
-  if (MFI.hasCalls() && MF.getTarget().Options.DisableFramePointerElim(MF))
     return true;
----------------
efriedma wrote:
> MaskRay wrote:
> > efriedma wrote:
> > > kamleshbhalui wrote:
> > > > MFI.hasCalls() was returning here false, Even for function who has call inside. May be because it is called earlier,before generating Full frame Info .  It needs to be removed from here. I could that it is done.
> > > Isn't TargetOptions::DisableFramePointerElim itself calling hasCalls()?  I guess we conservatively return true anyway if the frame info isn't computed yet, though, so probably doesn't matter.
> > TargetOptions::DisableFramePointerElim doesn't call hasCalls(). Deleting hasCalls() will make -frame-pointer=[non-leaf|all] different.
> > TargetOptions::DisableFramePointerElim doesn't call hasCalls()
> 
> https://github.com/llvm/llvm-project/blob/11448eeb72e1392f9f4ad072866c2c6dc82d14bc/llvm/lib/CodeGen/TargetOptionsImpl.cpp#L49
Thanks for the pointer. It looks there are other cleanup opportunities in TargetOptions::DisableFramePointerElim.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71168