[PATCH] D150416: [RISCV] Add a pass to combine `cm.pop` and `ret` insts

[Xinlong Wu via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/RISCV/RISCVPushPopOptimizer.cpp:140
+  // If frame pointer elimination has been disabled,
+  // abort to avoid breaking the ABI.
+  if (Fn.getTarget().Options.DisableFramePointerElim(Fn)) {
----------------
VincentWu wrote:
> jrtc27 wrote:
> > craig.topper wrote:
> > > jrtc27 wrote:
> > > > craig.topper wrote:
> > > > > VincentWu wrote:
> > > > > > craig.topper wrote:
> > > > > > > How does it break the ABI?
> > > > > > it was mentioned here https://github.com/plctlab/llvm-project/issues/58 ,
> > > > > > but I'm not sure whether it is the best solution
> > > > > This question was not answered.
> > > > https://github.com/riscv/riscv-code-size-reduction/issues/194 and https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/18#issuecomment-1304496707 onwards
> > > As far as I can tell, this patch just fuses the cm.pop with ret. Wouldn't the ABI break occur at the creation of cm.pop not at the formation of cm.popret?
> > Yes, using cm.push/pop is the issue, not fusing ones that already exist. Haven't looked at the content of the patch other than the few lines of context here.
> yes, So you can find the same code in the patch D134599#change-ZdluzfeA7hef
> 
> This code here is just a quick exit if it doesn't match the condition
> it was mentioned here https://github.com/plctlab/llvm-project/issues/58 ,
> but I'm not sure whether it is the best solution

Sorry for late reply, I forgot to submit it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150416