[PATCH] D101027: [CSKY 7/n] Add more basic instructions including some privilege instructions

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added inline comments.


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:208
+
+    std::pair<unsigned, unsigned> regSeq = getRegSeq();
+
----------------
As clang tidy suggested, I think local variables should be capitalized


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:210
+
+    fprintf(stderr, "regSeq = %d, %d\n", regSeq.first, regSeq.second);
+
----------------
Is this debug message? If that's the case please use `LLVM_DEBUG` macro and `errs()` / `dbgs()` stream


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:230
+    assert(Kind == RegisterSeq && "Invalid type access!");
+    return std::pair<unsigned, unsigned>(RegSeq.RegNumFrom, RegSeq.RegNumTo);
+  }
----------------
Can we use `std::make_pair` here? The current syntax is a little too verbose


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:672
+
+  auto Ry = Operands.back()->getReg();
+  Operands.pop_back();
----------------
Can we use explicit type here?


================
Comment at: llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp:687
+
+  auto Rz = Operands.back()->getReg();
+  Operands.pop_back();
----------------
ditto


================
Comment at: llvm/lib/Target/CSKY/MCTargetDesc/CSKYInstPrinter.cpp:101
+  auto Ry = MI->getOperand(OpNo).getReg();
+  auto Rz = MI->getOperand(OpNo + 1).getImm();
+
----------------
Can we use explicit type for Ry and Rz?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101027