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

[Alex Bradbury via llvm-commits]

asb marked an inline comment as done.

================
Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:66
@@ +65,2 @@
+
+def GPR32 : RegisterClass<"RISCV", [i32], 32, (add GPR)>;
----------------
asb wrote:
> 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.
Ok, having had a look at this some more I think I mostly answered my own question. When writing to a register in RV64 (e.g. with `ADDW`) we do indeed need to model in-register sign extension, which I think is similar to MIPS. But the operands to `ADDW` or `SUBW` come from truncating the 64-bit register. Arguably the most sensible way of modelling this is by having a 32-bit register which is a subregister of the matching 64-bit GPR.

What I really want is that almost all instructions are defined as taking register operands from the 'GPR' set, which depending on if the target is RV32 or RV64 may be GPR32 or GPR64 (or in the future with RV128, GPR128). A small number of instructions as `SLLW` are defined to take a 32-bit subregisters, and in the future with RV128 we would have `ADDD`, `SLLD` and friends which take the 64-bit subregister of one of the GPR128 register set.

I'll have more of a think about what I want to do here.


https://reviews.llvm.org/D23561