[PATCH] D40001: [RISCV] MC layer support for the load/store instructions of standard compress instruction set

[Shiva Chen via Phabricator via llvm-commits]

shiva0217 added a comment.

Hi @mgrang, @apazos

Thanks for the comments.
I update the patch using "-U 9999999999" and also using clang-format to check indentation.



================
Comment at: lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:87
+                                            const void *Decoder) {
+   if (RegNo > 8) {
+     return MCDisassembler::Fail;
----------------
apazos wrote:
> no need to add { } 
remove unnecessary {}.


================
Comment at: lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:96
+// Add Imply SP operand for SP imply instructions(The instruction SP not encode
+// in bit field)
+static void addImplySP(MCInst &Inst, int64_t Address, const void *Decoder) {
----------------
mgrang wrote:
> Period after comment.
Add period after comment.


================
Comment at: lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:149
+
+  // It's a 32 bit instruction if bit 0 and 1 is 1.
+  if ((Bytes[0] & 0x3) == 0x3) {
----------------
apazos wrote:
> if bit 0 and 1 are 1.
Fixup comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:87
+  const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
+  // Get byte count of instruction
+  unsigned Size = Desc.getSize();
----------------
mgrang wrote:
> Period after comment.
Add period after comment.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:98
+  }
+  case 2: {
+    uint16_t Bits = getBinaryCodeForInstr(MI, Fixups, STI);
----------------
mgrang wrote:
> Can you order the cases in the ascending order of Size?
Order the case by ascending order as @mgrang suggest.


https://reviews.llvm.org/D40001