About a new porting of GlobalIsel for RISCV

I would like to start a new porting of GlobalIsel for RISCV.
An initial patch about GlobalIsel infrastructure for RISCV was ready now:
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:
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
I have made some experiment that trying to implement the "LowerReturn"
function, and it can return correctly. The code snippet may be as
  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:
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.

Andrew Wei

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.

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.

Andrew Wei

Daniel Sanders <daniel_l_sanders@apple.com> 于2019年7月26日周五 上午4:00写道: