[Aarch64AsmPrinter] support %c output template for global addresses

[nickdesaulniers]

Summary

The Linux kernel makes use of %c<digit> "output template" in extended
inline assembly, particularly with asm goto for static key runtime
patching. The inline assembly uses the address of a global variable as
input to the extended inline assembly.

There's generic handling of %c in AsmPrinter::PrintAsmOperand() for
immediates. I considered extending support for global addresses there,
but it seems that X86AsmPrinter::PrintAsmOperand() has quite a bit of
custom logic for printing global addresses (see printSymbolOperand() in
X86AsmPrinter.cpp).

Fixes: https://bugs.llvm.org/show_bug.cgi?id=41402
Link: https://gcc.gnu.org/onlinedocs/gccint/Output-Template.html#Output-Template

[peter.smith]

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.

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

499

I think this comment will need updating to mention that the implementation of 'c' is split between AsmPrinter::PrintAsmOperand and this function,

509

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.

[nickdesaulniers]

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

Thinking more about this last night; it's going to be a pain/stupid to keep reimplementing this for other arches. The Linux kernel is using %c with global addresses for arm32 and ppc (and many others). I think I should:

1. Create AsmPrinter::printSymbolOperand(), a virtual method of the generic class, that simply forwards its arguments to the virtual method printAsmOperand().
2. Update X86AsmPrinter::printSymbolOperand() to be a member function rather than static local function, overriding the base AsmPrinter::printSymbolOperand().
3. Handle %c in the generic AsmPrinter::PrintAsmOperand(), deferring to the now virtual method printSymbolOperand() for MachineOperand::MO_GlobalAddress, thus supporting %c w/ MachineOperand::MO_GlobalAddress for all architectures.

How does that sound?

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

509

Right, this tripped me up when I made the comment in the bug (https://bugs.llvm.org/show_bug.cgi?id=41402#c3). It's called before the switch (L499) vs in the default case.

[nickdesaulniers]

How does that sound?

eh, looks like the arch specific asm printers have printOperand as well that would need to be hoisted to the base class in order to properly handle %c in a generic way. That will likely be a bigger refactor to AsmPrinter and derived classes. I think I'm going to do that in a separate commit first, then revisit this one.

[peter.smith]

In D60428#1460079, @nickdesaulniers wrote:

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

Thinking more about this last night; it's going to be a pain/stupid to keep reimplementing this for other arches. The Linux kernel is using %c with global addresses for arm32 and ppc (and many others). I think I should:

1. Create AsmPrinter::printSymbolOperand(), a virtual method of the generic class, that simply forwards its arguments to the virtual method printAsmOperand().
2. Update X86AsmPrinter::printSymbolOperand() to be a member function rather than static local function, overriding the base AsmPrinter::printSymbolOperand().
3. Handle %c in the generic AsmPrinter::PrintAsmOperand(), deferring to the now virtual method printSymbolOperand() for MachineOperand::MO_GlobalAddress, thus supporting %c w/ MachineOperand::MO_GlobalAddress for all architectures.

How does that sound?

That sounds reasonable to me. I think some backends such as Power and Hexagon have explicit handling for 'c' that does nothing:

case 'c': // Don't print "$" before a global var name or constant.
  break; // PPC never has a prefix.

Will be worth checking all the backends for custom handling.

[nickdesaulniers]

Superseded by https://reviews.llvm.org/D60887

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

 }
 
 bool AArch64AsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNum,
                                         unsigned AsmVariant,
                                         const char *ExtraCode, raw_ostream &O) {
   const MachineOperand &MO = MI->getOperand(OpNum);
 
   // First try the generic code, which knows about modifiers like 'c' and 'n'.
   if (!AsmPrinter::PrintAsmOperand(MI, OpNum, AsmVariant, ExtraCode, O))
     return false;
 
   // Does this asm operand have a single letter operand modifier?
   if (ExtraCode && ExtraCode[0]) {
     if (ExtraCode[1] != 0)
       return true; // Unknown modifier.
 
     switch (ExtraCode[0]) {
     default:
       return true; // Unknown modifier.
     case 'a':      // Print 'a' modifier
       PrintAsmMemoryOperand(MI, OpNum, AsmVariant, ExtraCode, O);
       return false;
     case 'w':      // Print W register
     case 'x':      // Print X register
       if (MO.isReg())
         return printAsmMRegister(MO, ExtraCode[0], O);
       if (MO.isImm() && MO.getImm() == 0) {
case 'q': // Print Q register.
           break;
         default:
           return true;
         }
         return printAsmRegInClass(MO, RC, false /* vector */, O);
       }
       printOperand(MI, OpNum, O);
       return false;
+    case 'c':
+      if (MO.getType() == MachineOperand::MO_GlobalAddress) {
+        printOperand(MI, OpNum, O);
+        return false;
+      }
+      return true;
     }
   }
 
   // According to ARM, we should emit x and v registers unless we have a
   // modifier.
   if (MO.isReg()) {
     unsigned Reg = MO.getReg();
 

llvm/test/CodeGen/AArch64/inline-asm-globaladdress.ll

-; RUN: llc < %s -mtriple aarch64-gnu-linux | FileCheck %s
-; RUN: llc < %s -mtriple arm64-apple-darwin | FileCheck %s
+; RUN: llc < %s -mtriple aarch64-gnu-linux | FileCheck %s --check-prefixes=CHECK,CHECK-LINUX
+; RUN: llc < %s -mtriple arm64-apple-darwin | FileCheck %s --check-prefixes=CHECK,CHECK-DARWIN
 
 target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
 
 ; CHECK-LABEL: test_inlineasm_globaladdress:
 ; CHECK: b {{_?}}test_symbol
 define void @test_inlineasm_globaladdress() {
   call void asm sideeffect "b $0", "i"(void ()* @test_symbol)
   ret void
 }
 
 ; CHECK-LABEL: test_inlineasm_globaladdress_offset:
 ; CHECK: b {{_?}}test_symbol+4
 define void @test_inlineasm_globaladdress_offset() {
   call void asm sideeffect "b $0", "i"(void ()* bitcast (i8* getelementptr (i8, i8* bitcast (void ()* @test_symbol to i8*), i64 4) to void ()*))
   ret void
 }
 
 declare void @test_symbol()
+
+; CHECK-LABEL: test_inlineasm_c_output_template:
+; CHECK-LINUX: .xword baz
+; CHECK-DARWIN: .quad _baz
+@baz = internal global i32 0, align 4
+define dso_local i32 @test_inlineasm_c_output_template() {
+  tail call void asm sideeffect ".quad ${0:c}", "i"(i32* nonnull @baz)
+  ret i32 43
+}