[PATCH] D60887: [AsmPrinter] refactor to support %c w/ GlobalAddress'

Wed Apr 24 08:59:27 PDT 2019

peter.smith added a comment.

In D60887#1472308 <https://reviews.llvm.org/D60887#1472308>, @nickdesaulniers wrote:

> > so I guess these backends are defaulted off or something?
>
> @srhines identified that AVR and RISCV are not in `LLVM_ALL_TARGETS` in `llvm/CMakeLists.txt`.  Should I send a patch to turn them on? (assuming the tests aren't red).

I think the AVR and RISCV backends are, or at least were, in "experimental" state so weren't included in LLVM_ALL_TARGETS. I thought that RISCV at least had been promoted out of experimental but I guess the backend owner (ASB I think) hasn't done that yet.

I've checked the patch and it looks good from the Arm/AArch64 perspective. Only thing I spotted was a change to the MSP340 that I didn't understand, but that could be intentional.

================
Comment at: llvm/lib/Target/MSP430/MSP430AsmPrinter.cpp:101
+    O << '#';
+    PrintSymbolOperand(MO, O);
     return;
----------------
This looks to have taken out the case when isMemOp is true (the removed code could output &). I don't know enough about MSP430 to know whether this is the right thing to do though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60887