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

[James Y Knight via llvm-commits]

jyknight added inline comments.


> asb wrote in RISCVInstPrinter.cpp:10
> Basic assembly printing is fine. I think it's more that the accepted pseudo-instructions are under-documented, relocations aren't documented etc. These don't pose an issue for basic printing.

I'd just note that the spec does not document any asm syntax, leaving it up to implementors' imagination. Still, yes, for basic printing, there's not much ambiguity.

> rv32i-valid.s:4
> +
> +addi ra, sp, 2 # CHECK: encoding: [0x93,0x00,0x21,0x00]
> +slti a0, a2, -20 # CHECK: encoding: [0x13,0x25,0xc6,0xfe]

Each of these lines should be like:

  # CHECK: addi ra, sp, 2 # encoding: [0x93,0x00,0x21,0x00]
  addi ra, sp, 2

So that it is actually checking the output of the the InstPrinter code in addition to the encoding.

https://reviews.llvm.org/D23564