[llvm-dev] About a new porting of GlobalIsel for RISCV

[Daniel Sanders via llvm-dev]

Hi Andrew,

I've reviewed https://reviews.llvm.org/D65219 and LGTM'd it as it looks like the normal boilerplate to me.

It looks like the main issue is the CallLowering side which unfortunately I don't know much about as I haven't dug into that on the GISel side. I would like to discourage CallLowering::ValueHandler replacements like MipsCallLowering::MipsHandler unless there's a strong reason for them though (subclassing CallLowering::ValueHandler is fine). As far as I can see MipsCallLowering::MipsHandler generally doesn't need to have different API's to CallLowering::ValueHandler and I'd say that all targets should generally be using the same interfaces.

For the sake of completeness the MipsCallLowering::MipsHandler API changes I saw were:
getStackAddress() takes a CCValAssign instead of Size and Offset but only actually used the Size and Offset. The calling code could just pass them in.
assignValueToReg() is mostly different because there's a FPR->GPR specific copy instruction. I never got around to working on it when I worked for MIPS but I think that ideally there'd just be COPY and the instruction differences would be handled by copyPhysReg() based on the reg classes for the src and dst.
assignValueToAddress() removed three of the arguments but it could just ignore them.

> On Jul 24, 2019, at 09:47, wei wei via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Hi,
> I would like to start a new porting of GlobalIsel for RISCV.
> An initial patch about GlobalIsel infrastructure for RISCV was ready now:
> https://reviews.llvm.org/D65219
> There is another porting patch https://reviews.llvm.org/D41653  posted
> by Leslie Zhai at the end of 2017. I have checked with Leslie about
> the status of this patch.He has stopped developing it since some
> questions need be resolved. There are some discussions about it:
> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120098.html
> http://lists.llvm.org/pipermail/llvm-dev/2018-August/125094.html
> Till now, I think we have a reasonable solution to continue the work,
> the implementation of GlobalIsel from Mips is a good example, which
> use target-specific "MipsCCState" and "MipsCallLowering::MipsHandler"
> to handle Call/Arguments/Return lowering.
> For RISCV, there's no "CCAssignFnForCall" or "CCAssignFnForReturn"
> functions defined, just like the solution in Mips, a new
> target-specific "ValueHandler" will be created to support
> calllowering.
> I have made some experiment that trying to implement the "LowerReturn"
> function, and it can return correctly. The code snippet may be as
> follows:
>  ...
>  CCState CCInfo(F.getCallingConv(), F.isVarArg(), MF, ArgLocs, F.getContext());
>  TLI.analyzeOutputArgs(MF, CCInfo, Outs, true, nullptr);
>  RISCVValueHandler RetHandler(MIRBuilder, MF.getRegInfo(), Ret);
>  RetHandler.handleArg(ArgLocs, RetInfos);
>  ...
> In order to reduce duplicated code as much as possible, and reuse part
> of code from "TargetLowering" for GlobalIsel, the access specifiers
> for some functions in RISCVTargetLowering need be changed, like
> "analyzeOutputArgs". I have made a separate patch for it:
> https://reviews.llvm.org/D65218
> Since Leslie is no longer working on GlobalIsel, I want to continue
> his work and make some contributions for RISCV target.
> The code base was changed a lot from 2017, so I restart a new
> GlobalIsel(RISCV) patch for reviewing, any suggestions or reviewing
> comments would be appreciated.
> 
> Thanks,
> Andrew Wei
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev