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

[Alex Bradbury via llvm-commits]

On 31 August 2016 at 02:49, Philip Reames <listmail at philipreames.com> wrote:
> 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.

Yes, given the MC-first approach I'm taking I decided to ignore
register allocation preference at this stage. As you suggest, I'd
rather introduce codegen concerns later. A comment to this effect
would seem to make sense though. Would you be happy with that James?

[Sidenote: these email responses don't seem to be making it to
phabricator - they should do, right?]

Alex