How to implement lowerReturn for poring GlobalISel to RISCV?

Project Infrastructure LLVM Dev List Archives
1

Hi Leslie. Although it's useful to think ahead, I'd really recommend
to try to incrementally build up support as was done in my RISC-V
SelectionDAGISel implementation and the GlobalISel tutorials. An
initial milestone might be generating a return from a void function,
then a function with a simple addition in its body and so on.

The code generator doesn't need to generate MRET/SRET/URET
instructions. Typically these will just be used in hand-written
assembly (you can find examples of usage in
https://github.com/riscv/riscv-pk for instance). It might make sense
in the future to define a function attribute for interrupt handlers
much like Mips (MIPS Function Attributes (Using the GNU Compiler Collection (GCC))),
but that would require co-ordination with the GCC developers.

So the good news is that return lowering should be much simpler than
you're currently thinking.

Best,

Alex

2

Hi Alex,

Thanks for your teaching!

I only implemented GlobalISel skeleton for RISCV target, built with no errors, is it suitable to review code now as Initial porting or wait for more APIs (for example: lowerReturn, getStackAddress) implemented?

I use `a return from a void function` testcase:

define void @f() {
ret void
}

$ llc -global-isel -march=riscv32 -stop-after=irtranslator return.ll -o return.mir

...

savePoint: ''
restorePoint: ''
fixedStack:
stack:
constants:
body: |
bb.1 (%ir-block.0):
MRET

...

But I think it is not correct, so please teach me how to implement `return lowering` in a simple way, thanks!

PS: as you suggested I built RISCV GNU toolchain using upstream repository:

$ /opt/riscv-gnu-git/bin/riscv32-unknown-elf-gcc -O0 -Wall -c return.c -o return-riscv32-gnu.o

^-- return.c is equivalent to above LLVM IR testcase

$ /opt/riscv-gnu-git/bin/riscv32-unknown-elf-objdump -d return-riscv32-gnu.o

return-riscv32-gnu.o: file format elf32-littleriscv

Disassembly of section .text:

00000000 <f>:
0: ff010113 addi sp,sp,-16
4: 00812623 sw s0,12(sp)
8: 01010413 addi s0,sp,16
c: 00000013 nop
10: 00c12403 lw s0,12(sp)
14: 01010113 addi sp,sp,16
18: 00008067 ret

And I merged master branch, is it ok? or use other branch?

$ /opt/riscv-gnu-git/bin/riscv32-unknown-elf-gcc -v
Using built-in specs.
COLLECT_GCC=/opt/riscv-gnu-git/bin/riscv32-unknown-elf-gcc
COLLECT_LTO_WRAPPER=/opt/riscv-gnu-git/libexec/gcc/riscv32-unknown-elf/8.0.0/lto-wrapper
Target: riscv32-unknown-elf
Configured with: ../configure --target=riscv32-unknown-elf --enable-languages=c --disable-shared --disable-threads --disable-multilib --disable-gdb --disable-libssp --with-newlib --with-arch=rv32ima --with-abi=ilp32 --prefix=/opt/riscv-gnu-git
Thread model: single
gcc version 8.0.0 20171215 (experimental) (GCC)

3

Hi LLVM developers,

Thank Daniel Sanders, Aditya Nandakumar and Justin Bogner's Tutorial[1]:
Head First into GlobalISel about how to port, and Aditya took BPF target as
a simple instance:

bool BPFCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
                                   const Value *Val, unsigned VReg) const {
   assert(!Val == !VReg && "Return value without a vreg");
   MIRBuilder.buildInstr(BPF::RET);
   return true;
}

But how to implement it for RISCV target?
https://github.com/xiangzhai/llvm/commit/c49146edbbf655e97727e22e4a87a020fb8da6e5

Because there are separate trap return instructions[2] per privilege level:
MRET, SRET, and URET. MRET is always provided, while SRET must be provided
if supervisor mode is supported. URET is only provided if user-mode traps
are supported. and David added RISCV privileged instructionsit[3], then
merged into upstream, it might be more complex than ARM[4] please give me
some hint, thanks a lot!

Hi Leslie. Although it's useful to think ahead, I'd really recommend
to try to incrementally build up support as was done in my RISC-V
SelectionDAGISel implementation and the GlobalISel tutorials. An
initial milestone might be generating a return from a void function,
then a function with a simple addition in its body and so on.

The code generator doesn't need to generate MRET/SRET/URET
instructions. Typically these will just be used in hand-written
assembly (you can find examples of usage in
GitHub - riscv-software-src/riscv-pk: RISC-V Proxy Kernel for instance). It might make sense
in the future to define a function attribute for interrupt handlers
much like Mips (MIPS Function Attributes (Using the GNU Compiler Collection (GCC))),
but that would require co-ordination with the GCC developers.

So the good news is that return lowering should be much simpler than
you're currently thinking.

Implemented lowerReturn in simple way https://github.com/xiangzhai/llvm/commit/0adf9aa558dd452f8e5bffa3f5b127c58f9d3a43

And compared with GNU toolchain:

return-riscv32-gnu.o: file format elf32-littleriscv

Disassembly of section .text:

00000000 <f>:
0: ff010113 addi sp,sp,-16
4: 00812623 sw s0,12(sp)
8: 01010413 addi s0,sp,16
c: 00000013 nop
10: 00c12403 lw s0,12(sp)
14: 01010113 addi sp,sp,16
18: 00008067 ret

return-riscv32-llvm.o: file format elf32-littleriscv

Disassembly of section .text:

00000000 <f>:
0: ff010113 addi sp,sp,-16
4: 00112623 sw ra,12(sp)
8: 00812423 sw s0,8(sp)
c: 01010413 addi s0,sp,16
10: 00812403 lw s0,8(sp)
14: 00c12083 lw ra,12(sp)
18: 01010113 addi sp,sp,16
1c: 00008067 ret

Thanks for your hint :slight_smile:

4

Hi Alex,

Thanks for your teaching!

I only implemented GlobalISel skeleton for RISCV target, built with no
errors, is it suitable to review code now as Initial porting or wait for
more APIs (for example: lowerReturn, getStackAddress) implemented?

From my perspective, I think it would be useful to get a little more

confidence about what you've implemented by at least adding support
for lowering simple ALU operations. That would make it easier for
someone who isn't an expert in GlobalISel (such as me) to review.

I use `a return from a void function` testcase:
<snip>

Based on your follow-up message, it seems you got this working.
Although comparing to gcc can be useful, I'd suggest that comparing to
RISC-V SelectionDAGISel may be more useful. e.g. just using llc and
-print-after-all or -debug-only=isel to see how the current codegen
works. The existing tests in test/Codegen/RISCV should also be useful.

Best,

Alex

5

Hi Alex,

Sorry to bother you for the holidays!

I am stuck at CC_RISCV[1] you removed RetCC_RISCV32 and CC_RISCV32 from RISCVCallingConv.td and implemented a custom calling convention[2] function owing to TableGen-based calling convention definitions to lack flexibility and require substantial boilerplate when you reach their limitations.

I asked the similar question[3] in the mailing list before, and Diana suggested that having a CCAssignFn available would make it easier to use the existing infrastructure. Because ValueHandler needs a CCAssignFn varaible for handling return not void value in lowerReturn.

Please lead me to give me some direction, thanks a lot!

[1] https://reviews.llvm.org/D39898

[2] https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#procedure-calling-convention

[3] http://lists.llvm.org/pipermail/llvm-dev/2017-November/119357.html

6

Hi Leslie. It seems the options are either:
1) Refactoring the existing CC_RISCV32 function to conform to the
CCAssignFn type. This would require stuffing the extra needed
information into a CCState subclass.
2) Pass a dummy AssignFn to the ValueHandler constructor and override
ValueHandler::assignArg.

Option 2) provides a straight-forward route to just getting something
working but isn't really a long-term solution. However, with a working
baseline implementation it's easier to explore and test alternative
approaches. If I were you, I'd probably start with 2) in order to make
immediate progress, but plan to change approach prior to merging
upstream.

Best,

Alex

7

Merry Christmas:) Thanks for your leading! I chose the second option https://github.com/xiangzhai/llvm/commit/d1b5bdbb2f8f4b5ad559b2a9662745d0fa9ef83d

发自我的iPad

------------------ Original ------------------