[llvm] [RISCV] Share ArgGPRs array between SelectionDAG and GISel. (PR #74152)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 4 08:30:19 PST 2023
================
@@ -986,6 +986,9 @@ bool CC_RISCV_FastCC(const DataLayout &DL, RISCVABI::ABI ABI, unsigned ValNo,
bool CC_RISCV_GHC(unsigned ValNo, MVT ValVT, MVT LocVT,
CCValAssign::LocInfo LocInfo, ISD::ArgFlagsTy ArgFlags,
CCState &State);
+
+ArrayRef<MCPhysReg> getArgGPRs();
----------------
topperc wrote:
> > GISel doesn't need the other lists like ArgFPR16s, , ArgFPR32s, ArgFPR64s, ArgVRs, etc. If we move ArgGPRs I think we should move those to, but I don't see any good reason to do that since GISel doesn't need them. We already exported `CC_RISCV`, `CC_RISCV_FastCC`, `CC_RISCV_GHC` functions from SelectionDAG so GISel could use them. Those are the primary users of these arrays.
>
>
>
> I didn't follow the process of GISel porting, is it because we haven't supported FP CC so we don't need these lists? Anyway, I won't block this PR if you think it's better to not move. :-)
The FP and vector register lists are only used by the CC_RISCV* functions. Those are called by both SelectionDAG and GISel. GISel only needs direct access to the GPR list due to the VarArg handling.
>
>
>
> >
>
> > Or alternatively, maybe I could add something like this that we could share with SelectionDAG and GISel?
>
> >
>
> > ```
>
> > unsigned RISCV::getVarArgsRegistersToSave(CCState &State) {
>
> > unsigned Idx = State.getFirstUnallocated(ArgRegs);
>
> > return (ArgRegs.size() - Idx);
>
> > }
>
> > ```
>
> This could be done. And, we can still see a lot of duplications in SelectionDAG and GISel, can these be shared too?
>
>
Never mind, this doesn't work because we need the list for the save loop in the VarArg code.
https://github.com/llvm/llvm-project/pull/74152
More information about the llvm-commits
mailing list