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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:3387
+  }
+  case RISCV::CM_POPRET:
+  case RISCV::CM_POPRETZ:
----------------
Why is this needed?


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp:208
+  unsigned Imm = MI->getOperand(OpNo).getImm();
+  RISCVZC::printRlist(Imm, O);
+}
----------------
This needs to obey `ArchRegNames` and print `x1` instead of `ra`, `x8` instead of `s0` etc. when requested.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp:224
+  if (Opcode == RISCV::CM_PUSH) {
+    Spimm *= -1;
+  }
----------------
`Spimm = -Spimm`;


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:493
+                                             const MCSubtargetInfo &STI) const {
+  MCOperand MO = MI.getOperand(OpNo);
+  assert(MO.isImm() && "Rlist operand must be immediate");
----------------
const MCOperand &


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:65
+    // 0~3 Reserved for EABI
+    return (Imm>=4) && (Imm <=15);
+  }];
----------------
Inconsistent spacing around `>=` and `<=`


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZc.td:181
 
+// Zcmp
+let Predicates = [HasStdExtZcmp], Defs = [X10, X11],
----------------
These need to be in a `DecoderNamespace` like I did for Zcmt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132819