[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 Alex, yes, the test case for operand should not x0 is missing. New revision updates the test case and adds GPRNonX0 register class for it.



================
Comment at: lib/Target/RISCV/RISCVInstrFormatsC.td:52
+
+// Immediate value encoding would different for each instruction
+// which should set by each subclass.
----------------
asb wrote:
> Perhaps phrase as something like "The immediate value encoding differs for each instruction, so each subclass is responsible for setting the appropriate bits in the Inst field." In fact, it would be even better to document the bits that must be set, e.g. Inst{12-10}.
Update the comment and specify the bits need to be set for each instruction.


================
Comment at: lib/Target/RISCV/RISCVInstrInfoC.td:113
+  let Inst{3-2} = imm{7-6};
+}
+
----------------
asb wrote:
> What about Inst{12}? I think we need more tests in rv32c-valid.s that verify the immediate encoding is correct.
The Inst{12} is set by RVInst16CI due to the bit encoding is the same for each instruction in CI format.


Repository:
  rL LLVM

https://reviews.llvm.org/D40001