[PATCH] D60428: [Aarch64AsmPrinter] support %c output template for global addresses

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I have a mild preference for following most of the other backends so that we don't split the handling of 'c' across the generic and the target specific, but I'm happy to let that pass if others prefer it this way.



================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:499
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
   if (!AsmPrinter::PrintAsmOperand(MI, OpNum, AsmVariant, ExtraCode, O))
     return false;
----------------
I think this comment will need updating to mention that the implementation of 'c' is split between AsmPrinter::PrintAsmOperand and this function, 


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:509
     default:
       return true; // Unknown modifier.
     case 'a':      // Print 'a' modifier
----------------
Most other Targets, with RISCV being the exception have a return AsmPrinter::PrintAsmOperand(MI, OpNum, AsmVariant, ExtraCode, O)); here. That would mean that we could have all the implementation of 'c' in the switch statement. This does duplicate the handling of immediates here, but it is more consistent with the other targets and does mean all the logic is in one place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60428