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

[wei wei via llvm-dev]

Hi, Daniel,
Thank you for your comments and suggestions.
I think you're right, we'd better not replace the existing
"ValueHandler" with target-specific ones.
The biggest problems I encountered was that in RISCV there's no
"CCAssignFnForCall" and "CCAssignFnForReturn" interface, which will be
one of the arguments for constructor function(ValueHandler). But it
seems this problem can be solved. After I implemented some APIs from
"CallLowering::ValueHandler" like "assignArg" and "assignValueToReg",
several simple cases can run successfully.
Currently, I need to do some experiments to verify the feasibility of
this method, and try to write as many cases as possible to cover
different scenarios. I am confident that I can complete a basic
implementation of GlobalIsel Calllowering for RISCV.

Thanks,
Andrew Wei

Daniel Sanders <daniel_l_sanders at apple.com> 于2019年7月26日周五 上午4:00写道：
>
> 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
>