[PATCH] D50496: [RISCV] Implment pseudo instructions for load/store from a symbol address.

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added a comment.

Hi, thanks a lot for the patch. A few comments inline.

Could you add some negative tests so the instructions reject symbols that are not bare?



================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:995
     return Name == "tail" || Name == "call";
   case 1:
     // lla rdest, imm
----------------
You will need to extend this case otherwise symbols named like registers fail to parse. I ran into this same problem! :)

That said, maybe we want to rethink this because your change adds a bunch of pseudos to this case.

```
t.s:1:8: error: immediate must be an integer in the range [-2048, 2047]
lb a0, zero
       ^
t.s:2:8: error: immediate must be an integer in the range [-2048, 2047]
sb a3, zero, a1
       ^
```


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1230
+///   TmpLabel: AUIPC rdest, %pcrel_hi(symbol)
+const MCExpr *RISCVAsmParser::emitLoadSymbolAddress(const MCExpr *Symbol,
+                                                    const MCOperand &TmpReg,
----------------
Suggestion: what about renaming it `emitLoadSymbolAddressHi`, it might look like the full symbol address will be computed otherwise. Then you can rename the new `emitLoadWithSymbol` to just `emitLoadSymbol`.


https://reviews.llvm.org/D50496