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

[Zixuan Wu via Phabricator via llvm-commits]

zixuan-wu added inline comments.


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:222
 
+  bool isUImm1() const { return isUImm<1>(); }
   bool isUImm2() const { return isUImm<2>(); }
----------------
rengolin wrote:
> This is getting too long, why can't you just use the template version directly?
Because the interface is defined in generated inc file like isUImm1(), isUImm2()...


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:283
 
-  bool isConstpoolSymbol() const {
+  template <unsigned MIN, unsigned MAX> bool isRegSeqTemplate() const {
+    if (!isRegisterSeq())
----------------
rengolin wrote:
> Do you really need this templated version?
For now only used in isRegSeq(), but also used in float register sequence predictor isRegSeqF1() which is not upstreamed now.


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:399
+      else
+        return "noreg";
+    };
----------------
rengolin wrote:
> Shouldn't this be an error? Or an assert?
I think it's a print result, it may not make sense to assert to debug.


================
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>;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111701