[PATCH] D111701: [CSKY] Complete to add basic integer instruction set

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: llvm/lib/Target/CSKY/CSKYInstrInfo.td:312
   def ORI32 : I_16_ZX<"ori32", uimm16,
-  [(set GPR:$rz, (or GPR:$rx, uimm16:$imm16))]>;
+  [(set i32:$rz, (or i32:$rx, uimm16:$imm16))]>;
   def XORI32 : I_12<0x4, "xori32", xor, uimm12>;
----------------
zixuan-wu wrote:
> rengolin wrote:
> > zixuan-wu wrote:
> > > rengolin wrote:
> > > > Sorry, I'm missing the context... Why isn't this a GPR? Is `i32` a 32-bit view of the register (like AArch64)?
> > > > 
> > > > If so, then `i32` is confusing because of the IR types. Perhaps `GPR32`?
> > > because GPR is also for DSP instruction so that it can also contain v4i8 and v2i16 type.
> > From `CSKYRegisterInfo.td`, `GPR` lists only `R*` registers. Do those alias with DPS registers?
> > 
> > Other targets keep `GPR` as scalar registers and have other naming for registers that can alias FP/Vector registers.
> > 
> > For example, AArch64 names `GPR64` and `GPR32` (and many other classes) to deal with those issues.
> > 
> > You're better off naming the classes now and specialising later, than just use the bit-width. This makes it much simpler to read the table-gen files and also maintain them when they grow too large.
> If I understand correctly, you prefer to use another register class such as DSPR to represent v4i8 and v2i16 even if it also contains general register R0-R31 as GPR?
Yes, that's the idea. Sorry for going around in circles. :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111701/new/

https://reviews.llvm.org/D111701