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

[Alex Bradbury via Phabricator via llvm-commits]

asb requested changes to this revision.
asb added a comment.
This revision now requires changes to proceed.

Added a couple of comments. Once addressed, should be fine. Thanks!



================
Comment at: lib/Target/RISCV/RISCVAsmPrinter.cpp:118
 
-    OS << "0(" << RISCVInstPrinter::getRegisterName(MO.getReg()) << ")";
+    OS << "(" << RISCVInstPrinter::getRegisterName(MO.getReg()) << ")";
     return false;
----------------
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)
```


================
Comment at: lib/Target/RISCV/RISCVISelDAGToDAG.cpp:175
     return false;
+  case InlineAsm::Constraint_A:
+    OutOps.push_back(Op);
----------------
Doesn't need to be an extra case given the body is the same as the others.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54296