[PATCH] D54296: [RISCV] Lower inline asm constraint A for RISC-V

[James Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: lib/Target/RISCV/RISCVAsmPrinter.cpp:118
 
-    OS << "0(" << RISCVInstPrinter::getRegisterName(MO.getReg()) << ")";
+    OS << "(" << RISCVInstPrinter::getRegisterName(MO.getReg()) << ")";
     return false;
----------------
asb wrote:
> Why make this change? It doesn't seem to match the GNU tools default, which I'd rather we stay in line with.
> 
> ```
> $ cat t.s 
> lw a0, 0(a0)
> $ ./riscv32-unknown-elf-as t.s
> $ ./riscv32-unknown-elf-objdump -d a.out 
> 
> a.out:     file format elf32-littleriscv
> 
> 
> Disassembly of section .text:
> 
> 00000000 <.text>:
>    0:	00052503          	lw	a0,0(a0)
> ```
Because we can’t differentiate between A and m constraints here. A has no immediate and is suitable for all loads and stores, including atomics, so GCC will emit just `(reg)` matching the canonical form for atomics (GNU as also accepts an immediate that evaluates to zero and discards it). Whereas m can have an immediate, though we hard-coded it to 0 here. We accept `lw x1, (x2)` but we don’t accept (or at least certainly didn’t used to accept) `lr.w x1, 0(x2)`, so we have to emit the form that works for everything, even if it’s not canonical for immediate loads and stores.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54296