[PATCH] D115224: [RISCV] Support named opcodes in .insn directive.

[Nelson Chu via Phabricator via llvm-commits]

Nelson1225 marked 3 inline comments as done.
Nelson1225 added a comment.

Thanks for your review and suggestions, Craig and Jessica.  All fixed.

Nelson



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:183
+}
+def uimm7_opcode : Operand<XLenVT> {
+  let ParserMatchClass = InsnDirectiveOpcode;
----------------
jrtc27 wrote:
> Needs a line between these
Thanks, fixed.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZb.td:385
 let Predicates = [HasStdExtZbb] in {
-def CLZ  : RVBUnary<0b0110000, 0b00000, 0b001, RISCVOpcode<0b0010011>, "clz">,
+def CLZ  : RVBUnary<0b0110000, 0b00000, 0b001, OPC_OP_IMM, "clz">,
            Sched<[WriteCLZ, ReadCLZ]>;
----------------
craig.topper wrote:
> I copied these changes to D115172 from your internal code review before you posted this. They've been committed now along with moving the line break to after the memonic string. Can you rebase this patch?
Sure, thanks for the reminder.


================
Comment at: llvm/test/MC/RISCV/insn-invalid.s:25
+# Unrecognized opcode name
+.insn r UNKNOWN, 0, a1, a2, a3 #CHECK: :[[@LINE]]:9: error: operand must be a valid opcode name or an integer in the range [0, 127]
----------------
jrtc27 wrote:
> Group these with the other invalid operand tests? i.e. leave the `.insn_i` test as a special case at the end of the file
Agreed, move the new testcase forward, and leave the .insn_i test at the end looks better.


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

https://reviews.llvm.org/D115224