[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