This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Initial porting GlobalISel
AbandonedPublic

Authored by xiangzhai on Dec 31 2017, 9:07 PM.

Details

Reviewers
asb
Summary

Hi LLVM developers,

Motivation:

I am learning and practicing Instruction Selection by porting GlobalISel to RISCV target.

Status:

  • Pass a dummy CCAssignFn to the ValueHandler constructor and override ValueHandler::assignArg, and it is better to refactory the existing CC_RISCV32 function to conform to the CCAssignFn type in the next patch.
  • RegisterBank supports 32bit and 64bit for FPR , but it only supports 32bit for General Purpose Register, because GPR64 had been merged into GPR in rL316159 and GlobalISel needs to consider about how to support variable-sized register classes concept implemented in D24631.
  • Legalizer and InstructionSelector only cover some instructions, so if run testcase, for example, calling-conv.ll, it will throw such errors:
LLVM ERROR: unable to legalize instruction: %1:_(s128) = G_SITOFP %0:_(s64); (in function: caller_mixed_scalar_libcalls)

It is an initial porting, so it is just able to run very simple testcase:

$ cat return.ll

define void @f0() {
  ret void
}

define i32 @f1() {
  ret i32 0
}

define i32 @f2() {
  %1 = call i32 @f1()
  ret i32 %1
}

$ llc -global-isel -march=riscv32 return.ll -o return-riscv32-global-isel.s

f:
	addi	sp, sp, -16
	sw	ra, 12(sp)
	sw	s0, 8(sp)
	addi	s0, sp, 16
	lw	s0, 8(sp)
	lw	ra, 12(sp)
	addi	sp, sp, 16
	ret

f1:
	addi	sp, sp, -16
	sw	ra, 12(sp)
	sw	s0, 8(sp)
	addi	s0, sp, 16
	lw	s0, 8(sp)
	lw	ra, 12(sp)
	addi	sp, sp, 16
	ret

f2:
	addi	sp, sp, -16
	sw	ra, 12(sp)
	sw	s0, 8(sp)
	addi	s0, sp, 16
	jalr	ra, f1, 0
	lw	s0, 8(sp)
	lw	ra, 12(sp)
	addi	sp, sp, 16
	ret

Please review my patch, and give me some suggestion, thanks for your teaching!

Regards,
Leslie Zhai

Diff Detail

Repository
rL LLVM

Event Timeline

xiangzhai created this revision.Dec 31 2017, 9:07 PM
asb added a comment.Jan 18 2018, 3:52 AM

Hi Leslie, this patch includes rather a lot which makes it somewhat hard to review - i.e. you're doing call handling, loads/store selection, selection for some ALU ops, FPR32/FPR64 regs + fcmp, and there aren't any tests attached that check this all works.

Do you think it might make sense to start with something more minimal, perhaps stripping this back to doing the bare minimum for a simple function - e.g. a ret void no-op, or a function that adds two arguments. I think that would make it much easier to review and move this forwards. Many thanks for your work here.

Hi Alex,

Thanks for your pointing out my fault clearly!

The patch is only able to work for such simple testcase:

define void @f0() {
  ret void
}

define i32 @f1() {
  ret i32 0
}

define i32 @f2() {
  %1 = call i32 @f1()
  ret i32 %1
}

But failed to work for testcase "test/CodeGen/RISCV/calling-conv.ll", so I need to implement D41700 and D41774 firstly, then I will rebase this patch.

PS: there is no response about RegisterBank in llvm-dev ML, and I have to cover ALL instructions for Legalizer and InstructionSelector.

Regards,
Leslie Zhai

lenary added a subscriber: lenary.Aug 1 2019, 5:09 AM

Given there is a better initial GlobalISel patch here: D65219, which matches the minimal approach @asb suggested, please can you mark this patch as "abandoned"?

Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2019, 5:09 AM
xiangzhai abandoned this revision.Jun 17 2020, 2:31 AM