[PATCH] D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 28 11:43:06 PST 2021


MaskRay added inline comments.


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:124
+  bool isImm() const override { return Kind == Immediate; }
+  bool isMem() const override { return false; }
+
----------------
Is `isMem` unimplemented? If yes, deferring to a subsequent patch is better.


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:283
+    ListSeparator LS;
+    std::string Msg = "instruction requires the following:";
+    for (unsigned i = 0, e = MissingFeatures.size(); i != e; ++i) {
----------------
The convention is to have a space after `:`


================
Comment at: llvm/test/MC/CSKY/basic.s:1
+# RUN: llvm-mc %s -triple=csky -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK-ASM %s
----------------
The joined line is sufficiently short, no need for wrapping.


================
Comment at: llvm/test/MC/CSKY/basic.s:98
+
+## uimm12/oimm12/nimm12
+addi32 a0, a0, 0 # CHECK: :[[#@LINE]]:16: error: immediate must be an integer in the range [1, 4096]
----------------
If you use `## ` for non-RUN non-CHECK comment markers, please stick with the rule consistently for subsequent comments.


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

https://reviews.llvm.org/D93798



More information about the llvm-commits mailing list