[llvm] [RISCV] Stash GPR to FPR if emergency spill slot is not reachable (PR #180685)

[via llvm-commits]

LukeZhuang wrote:

> > > > > This seems like a hack. It doesn't work when the D extension isn't present.
> > > > > I think the patch below is the beginning of a better fix, but it causes other issues that need to be worked through. Ignore the mention of ARM. This was copy and pasted as an experiment to fix #180199.
> > > > > ```
> > > > > @@ -1860,7 +1864,16 @@ void RISCVFrameLowering::processFunctionBeforeFrameFinalized(
> > > > >  // by the frame pointer.
> > > > >  // Let eliminateCallFramePseudoInstr preserve stack space for it.
> > > > >  bool RISCVFrameLowering::hasReservedCallFrame(const MachineFunction &MF) const {
> > > > > -  return !MF.getFrameInfo().hasVarSizedObjects() &&
> > > > > +  const MachineFrameInfo &MFI = MF.getFrameInfo();
> > > > > +  unsigned CFSize = MFI.getMaxCallFrameSize();
> > > > > +  // It's not always a good idea to include the call frame as part of the
> > > > > +  // stack frame. ARM (especially Thumb) has small immediate offset to
> > > > > +  // address the stack frame. So a large call frame can cause poor codegen
> > > > > +  // and may even makes it impossible to scavenge a register.
> > > > > +  if (CFSize >= ((1 << 12) - 1) / 2)  // Half of imm12
> > > > > +    return false;
> > > > > +
> > > > > +  return !MFI.hasVarSizedObjects() &&
> > > > >           !(hasFP(MF) && hasRVVFrameObject(MF));
> > > > >  }
> > > > > ```
> > > > 
> > > > 
> > > > Yes you are right. It has some issue such as requires D extension and the issue still exists if FPR is also used up. While I guess it's okay since it can be a workaround for rv64gc for now and be replaced by a more elegant solution later. PowerPC does the similar thing so I think it's not quite tricky.
> > > 
> > > 
> > > What is the urgency to do this instead of a proper fix?
> > 
> > 
> > It's not urgent, we just found the issue and try to resolve it. But may I ask why do you think it is "not a proper fix"? It can solve part of the issue including our one, and also other targets did the same way.
> 
> As you noted, it doesn't work without D, or on RV32, or if the FPRs are exhausted. To me that isn't fixing the underlying issue. It's just hiding it in some cases.
> 
> Reading the PowerPC conversation, it sounds like they handle their frame pointer differently than RISC-V. So I don't think the RISC-V issue has the same underlying cause as PowerPC. So I think we should consider if we can use the frame pointer to fix this issue on RISC-V.

Sure, I see your concern here. I agree it's not perfect and is more like a workaround when we have F/D extension. I'm happy to see if there is a more elegant and perfect solution.
Before that, I can keep it open. I just modified this patch and added FPR32 part, it can deal with RV32 now, but still, only works when F extension exists :)

https://github.com/llvm/llvm-project/pull/180685