[PATCH] D23561: [RISCV 4/10] Add basic RISCV{InstrFormats, InstrInfo, RegisterInfo, }.td

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 31 05:25:23 PDT 2016

asb marked an inline comment as done.

Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:28
@@ +27,3 @@
+let RegAltNameIndices = [ABIRegAltName] in {
+  def X0    : RISCVReg<0, "x0", ["zero"]>, DwarfRegNum<[0]>;
+  def X1    : RISCVReg<1, "x1", ["ra"]>, DwarfRegNum<[1]>;
They're not. I have added that to the list.

Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:64
@@ +63,3 @@
+  (sequence "X%u", 0, 31)
There have been some responses to this that haven't been picked up by phabricator: [1](http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160829/386399.html), [2](http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160829/386425.html).

To restate the discussion there - I would like to initially focus on MC concerns in this patchset and only introduce codegen concerns when they can be tested. Would you be happy with a comment here explaining that the allocation order should be changed when codegen is implemented?

Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:66
@@ +65,2 @@
+def GPR32 : RegisterClass<"RISCV", [i32], 32, (add GPR)>;
Could you elaborate on where things go wrong for SPARC? If it's conceptual failing of having a single set of register IDs, what precisely is the issue? There doesn't for instance seem to be an assumption that any given `Register` is a member of only a single `RegisterClass`. Or is it likely due to bugs in tablegen or elsewhere?

In PPC and AArch64, the 32-bit registers are defined as subregisters of the 64-bit GPRs but I'm not sure if this is as correct for RISC-V. The compiler is either targeting RV32 or RV64, and when targeting RV64 there is no instruction that will modify only 32-bits of the register. Values are always held in sign-extended format so bit 31 will be copied in bits 32-63 if you for instance execute an `ADDW` or one of the other RV64 instructions defined to work on 32-bit quantities. Load instructions defined to either zero bits 32-63 or to sign-extend the loaded value.


More information about the llvm-commits mailing list