[PATCH] D132819: [RISCV] Add MC support of RISCV zcmp Extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1048
+  static std::unique_ptr<RISCVOperand> createRlist(unsigned RlistEncode,
+                                                   SMLoc S, bool IsRV64) {
+    auto Op = std::make_unique<RISCVOperand>(KindTy::Rlist);
----------------
Drop IsRV64


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1056
+  static std::unique_ptr<RISCVOperand> createSpimm(unsigned Spimm, SMLoc S,
+                                                   bool IsRV64) {
+    auto Op = std::make_unique<RISCVOperand>(KindTy::Spimm);
----------------
Drop IsRV64


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1473
+        ErrorLoc,
+        "This stack adjustment is invalid for this instruction and register "
+        "list, "
----------------
Drop the word "This" from the beginning. Drop the word "Please".

Something like

`stack adjustment is invalid for this instruction and register; refer to Zc spec for a detailed range of stack adjustment`


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2323
+  if (getLexer().isNot(AsmToken::LCurly)) {
+    Error(getLoc(), "Rlist must start with '{'");
+    return MatchOperand_ParseFail;
----------------
Rlist -> register list


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2323
+  if (getLexer().isNot(AsmToken::LCurly)) {
+    Error(getLoc(), "Rlist must start with '{'");
+    return MatchOperand_ParseFail;
----------------
craig.topper wrote:
> Rlist -> register list
General comment, error messages should not start with capitalized words.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2352
+    if (RegStart != RISCV::X8) {
+      Error(getLoc(), "Continuous registers list must start from 's0' or 'x8'");
+      return MatchOperand_ParseFail;
----------------
registers -> register


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2369
+      Error(getLoc(),
+            "Contiguous registers list of EABI only can be 's0-s1' or "
+            "'x8-x9' pair");
----------------
This sentence is awkward.

Something like

"contiguous register list for EABI can only be 's0-1' or 'x8-x9'"


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2381
+        Error(getLoc(),
+              "First contiguous registers pair of XRlist must be 'x8-x9'");
+        return MatchOperand_ParseFail;
----------------
XRlist -> register list


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2395
+            getLoc(),
+            "Second contiguous registers pair of XRlist must start from 'x18'");
+        return MatchOperand_ParseFail;
----------------
XRList -> register list


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2425
+  if (getLexer().isNot(AsmToken::RCurly)) {
+    Error(getLoc(), "Rlist must end with '}'");
+    return MatchOperand_ParseFail;
----------------
Rlist -> register list


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2435
+  if (Encode == 16) {
+    Error(S, "invalid Register list");
+    return MatchOperand_ParseFail;
----------------
Don't capitalize Register


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819