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

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 30 18:49:30 PDT 2016


On 08/26/2016 09:39 PM, James Y Knight wrote:
> jyknight added inline comments.
>
> ================
> Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:27
> @@ +26,3 @@
> +// Integer registers
> +let RegAltNameIndices = [ABIRegAltName] in {
> +  def X0    : Ri<0, "x0", ["zero"]>, DwarfRegNum<[0]>;
> ----------------
> Are the risc-v dwarf register numbers documented anywhere? (I assume not, just like the relocation types, but thought I'd ask anyways. Maybe you can start a list of Things That Ought To Be Documented...)
>
> ================
> Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:63
> @@ +62,3 @@
> +def GPR : RegisterClass<"RISCV", [i64], 64, (add
> +  (sequence "X%u", 0, 31)
> +)>;
> ----------------
> May want to use a different register allocation sequence.
Wait, this also controls register allocation order preference?  Hm, 
would not have guessed that from the bit of backend code I'd read. With 
that in mind, I'd prefer we submit with a simple structure and then land 
a separate change with clear comments changing the allocation order.
> Typically backends put caller-save registers first, then callee save. In RISCV, we probably also want to prefer registers in the range X8-X15, since they're usable by the compressed instructions.
>
> How about putting the registers in the order:
> X10-X17 [a0-a7] , X5-X7 [t0-t2], X28-X31 [t3-t6], X8-X9 [s0-s1], X18-X27 [s2-s11], X0-X4 [zero, ra, sp, gp, tp]
>
> That is: caller-save between x10-15, then the remaining caller-save, then callee-save, then specials (which will be reserved, anyways)
>
> ================
> Comment at: lib/Target/RISCV/RISCVRegisterInfo.td:66
> @@ +65,2 @@
> +
> +def GPR32 : RegisterClass<"RISCV", [i32], 32, (add GPR)>;
> ----------------
> I think using one set of registers for both the 32-bit arch and the 64-bit arch doesn't actually work right -- you rather want separate 32bit and 64bit versions of the registers (perhaps name them X{0..31}_32 and X{0..31}_64 for clarity), to make up the GPR32 and GPR64 register classes.
>
> The SPARC backend actually does something like you have here, but I'm pretty sure its 64-bit support is fairly busted and is the wrong way to do things -- better to look at, perhaps, the PPC backend. Could name the registers X{0..31}_32 and X{0..31}_64.
>
>
>
> https://reviews.llvm.org/D23561
>
>
>



More information about the llvm-commits mailing list