[PATCH] D23564: [RISCV 7/10] Add RISCVInstPrinter and basic MC assembler tests

[Alex Bradbury via llvm-commits]

asb added inline comments.


================
Comment at: test/MC/RISCV/rv32i-valid.s:7
+addi ra, sp, 2 # CHECK: encoding: [0x93,0x00,0x21,0x00]
+# CHECK-INST: addi ra, sp, 2
+slti a0, a2, -20 # CHECK: encoding: [0x13,0x25,0xc6,0xfe]
----------------
jyknight wrote:
> If you flip this order around, putting the "CHECK-INST" first, then the "CHECK: encoding", you wouldn't need to run llvm-mc twice for each processor mode, once with show-encoding and once without; just give FileCheck both prefixes in one run.
> 
> I'd also suggest moving the encoding check up to its own line, I think it'll be more readable that way. e.g.:
> 
> ```
> # RUN: llvm-mc %s -triple=riscv32 -show-encoding | FileCheck -check-prefixes=CHECK,CHECK-INST %s
> # RUN: llvm-mc %s -triple=riscv64 -show-encoding | FileCheck -check-prefixes=CHECK,CHECK-INST %s
> 
> # CHECK-INST: addi ra, sp, 2
> # CHECK: encoding: [0x93,0x00,0x21,0x00]
> addi ra, sp, 2
> ```
That's definitely nicer - thanks! I've switched to using this style.


https://reviews.llvm.org/D23564