[Arm64EC] Refer to dllimport'ed functions correctly.

[efriedma]

Summary

Arm64EC has two different ways to refer to dllimport'ed functions in an object file. One is using the usual __imp_ prefix, the other is using an Arm64EC-specific prefix __imp_aux_. As far as I can tell, if a function is in an x64 DLL, __imp_aux_ refers to the actual x64 address, while __imp_ points to some linker-generated code that calls the exit thunk. So __imp_aux_ is used to refer to the address in non-call contexts, while __imp_ is used for calls to avoid the indirect call checker.

There's one twist to this, though: if an object refers to a symbol using the __imp_aux_ prefix, the object file's symbol table must also contain the symbol with the usual __imp_ prefix. The symbol doesn't actually have to be used anywhere, it just has to exist; otherwise, the linker's symbol lookup in x64 import libraries doesn't work correctly. Currently, this is handled by emitting a .globl __imp_foo directive; we could try to design some better way to handle this.

One minor quirk I haven't figured out: apparently, in Arm64EC mode, MSVC prefers to use a linker-synthesized stub to call dllimport'ed functions, instead of branching directly. The linker stub appears to do the same thing that inline code would do, so not sure if it's just a code-size optimization, or if the synthesized stub can actually do something other than just load from the import table in some circumstances.

[bcl5980]

I am OK with this change but only 1 question:
https://godbolt.org/z/Ths69E83j
MSVC will generate both __imp_aux_fn and __imp_fn even if we only use __imp_aux_fn. But it looks based on this change we can't do that. And it will cause link error based on my local test.

[efriedma]

Just listing the symbol in the symbol table doesn't do anything useful as far as I know. What's actually referring to __imp_importf? What do you mean by link errors?

[bcl5980]

In D136202#3868691, @efriedma wrote:

Just listing the symbol in the symbol table doesn't do anything useful as far as I know. What's actually referring to __imp_importf? What do you mean by link errors?

Nothing refer to __imp_fn. But that is an external symbol. I guess link will use it to generate the definition of __imp_aux_fn. So, if we don't export __imp_fn, link will report:
error LNK2019: unresolved external symbol __imp_aux_fn referenced in function #p (EC Symbol)

[efriedma]

Strange, I didn't see anything like that. Are you sure that adding the symbol to the symbol table solves the issue?

I did see a few weird issues with certain C library functions, but it turned out I could reproduce the same issue with MSVC.

[bcl5980]

In D136202#3870050, @efriedma wrote:

Strange, I didn't see anything like that. Are you sure that adding the symbol to the symbol table solves the issue?

I did see a few weird issues with certain C library functions, but it turned out I could reproduce the same issue with MSVC.

Yeah, for example the arm64ec.exe source code is:

#include <stdio.h>

int __declspec(dllimport) fn(int a);

int main(int argc, const char* argv[])
{
    printf("result:%p\n", (void*)fn);
    return 0;
}

We will only create symbol __imp_aux_fn for now.

The x64 dll source code is:

int __declspec(dllexport) fn(int a)
{
    return a + 1;
}

The x64 dll only export the symbol __imp_fn.
So, we should export __imp_fn in arm64ec.obj I think.

[efriedma]

The way you're describing the issue doesn't make sense. Both __imp_ and __imp_aux_ are "magic" prefixes; they only exist in object files, not linked products (dll/exe).

That said, if you link an arm64ec binary against an x64 library, link.exe does in fact appear to fail to resolve __imp_aux_fn unless __imp_fn is also in the symbol table. Note this only applies to linking against x64 libraries; linking against arm64ec libraries works fine (which is why I didn't run into issues when I was testing this). The fact that the behavior depends on how the library is compiled is so inexplicably weird that I have to call it a bug... but Microsoft probably isn't going to help us here, so we probably need to come up with a workaround.

[bcl5980]

In D136202#3870113, @efriedma wrote:

The way you're describing the issue doesn't make sense. Both __imp_ and __imp_aux_ are "magic" prefixes; they only exist in object files, not linked products (dll/exe).

The __imp_ prefix symbol also exist in lib files. Link.exe use the lib file as input to link dll.

That said, if you link an arm64ec binary against an x64 library, link.exe does in fact appear to fail to resolve __imp_aux_fn unless __imp_fn is also in the symbol table. Note this only applies to linking against x64 libraries; linking against arm64ec libraries works fine (which is why I didn't run into issues when I was testing this). The fact that the behavior depends on how the library is compiled is so inexplicably weird that I have to call it a bug... but Microsoft probably isn't going to help us here, so we probably need to come up with a workaround.

Yeah, that is exactly what I find, ARM64EC binary link a x64 library has this issue.

[efriedma]

Updated with the symbol table quirk.

[bcl5980]

LGTM

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

if (getTargetMachine().getCodeModel() == CodeModel::Large) {
     Result = getAddrLarge(GN, DAG, OpFlags);
   } else if (getTargetMachine().getCodeModel() == CodeModel::Tiny) {
     Result = getAddrTiny(GN, DAG, OpFlags);
   } else {
     Result = getAddr(GN, DAG, OpFlags);
   }
   EVT PtrVT = getPointerTy(DAG.getDataLayout());
   SDLoc DL(GN);
-  if (OpFlags & (AArch64II::MO_DLLIMPORT | AArch64II::MO_COFFSTUB))
+  if (OpFlags & (AArch64II::MO_DLLIMPORT | AArch64II::MO_DLLIMPORTAUX |
+                 AArch64II::MO_COFFSTUB))
     Result = DAG.getLoad(PtrVT, DL, DAG.getEntryNode(), Result,
                          MachinePointerInfo::getGOT(DAG.getMachineFunction()));
   return Result;
 }
 
 /// Convert a TLS address reference into the correct sequence of loads
 /// and calls to compute the variable's address (for Darwin, currently) and
 /// return an SDValue containing the final node.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

AArch64InstrInfo::getSerializableBitmaskMachineOperandTargetFlags() const {
 
   static const std::pair<unsigned, const char *> TargetFlags[] = {
       {MO_COFFSTUB, "aarch64-coffstub"},
       {MO_GOT, "aarch64-got"},
       {MO_NC, "aarch64-nc"},
       {MO_S, "aarch64-s"},
       {MO_TLS, "aarch64-tls"},
       {MO_DLLIMPORT, "aarch64-dllimport"},
+      {MO_DLLIMPORTAUX, "aarch64-dllimportaux"},
       {MO_PREL, "aarch64-prel"},
       {MO_TAGGED, "aarch64-tagged"}};
   return makeArrayRef(TargetFlags);
 }
 
 ArrayRef<std::pair<MachineMemOperand::Flags, const char *>>
 AArch64InstrInfo::getSerializableMachineMemOperandTargetFlags() const {
   static const std::pair<MachineMemOperand::Flags, const char *> TargetFlags[] =

llvm/lib/Target/AArch64/AArch64MCInstLower.cpp

 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
 #include "llvm/Target/TargetMachine.h"
 using namespace llvm;
 
 extern cl::opt<bool> EnableAArch64ELFLocalDynamicTLSGeneration;
 
 AArch64MCInstLower::AArch64MCInstLower(MCContext &ctx, AsmPrinter &printer)
     : Ctx(ctx), Printer(printer) {}
 
 MCSymbol *
 AArch64MCInstLower::GetGlobalAddressSymbol(const MachineOperand &MO) const {
   const GlobalValue *GV = MO.getGlobal();
   unsigned TargetFlags = MO.getTargetFlags();
   const Triple &TheTriple = Printer.TM.getTargetTriple();
   if (!TheTriple.isOSBinFormatCOFF())
     return Printer.getSymbolPreferLocal(*GV);
 
   assert(TheTriple.isOSWindows() &&
          "Windows is the only supported COFF target");
 
-  bool IsIndirect = (TargetFlags & (AArch64II::MO_DLLIMPORT | AArch64II::MO_COFFSTUB));
+  bool IsIndirect =
+      (TargetFlags & (AArch64II::MO_DLLIMPORT | AArch64II::MO_DLLIMPORTAUX |
+                      AArch64II::MO_COFFSTUB));
   if (!IsIndirect)
     return Printer.getSymbol(GV);
 
   SmallString<128> Name;
-  if (TargetFlags & AArch64II::MO_DLLIMPORT)
+
+  if (TargetFlags & AArch64II::MO_DLLIMPORTAUX) {
+    // __imp_aux is specific to arm64EC; it represents the actual address of
+    // an imported function without any thunks.
+    //
+    // If we see a reference to an "aux" symbol, also emit a reference to the
+    // corresponding non-aux symbol.  Otherwise, the Microsoft linker behaves
+    // strangely when linking against x64 import libararies.
+    //
+    // emitSymbolAttribute() doesn't have any real effect here; it just
+    // ensures the symbol name appears in the assembly without any
+    // side-effects. It might make sense to design a cleaner way to express
+    // this.
+    Name = "__imp_";
+    Printer.TM.getNameWithPrefix(Name, GV,
+                                 Printer.getObjFileLowering().getMangler());
+    MCSymbol *ExtraSym = Ctx.getOrCreateSymbol(Name);
+    Printer.OutStreamer->emitSymbolAttribute(ExtraSym, MCSA_Global);
+
+    Name = "__imp_aux_";
+  } else if (TargetFlags & AArch64II::MO_DLLIMPORT) {
     Name = "__imp_";
-  else if (TargetFlags & AArch64II::MO_COFFSTUB)
+  } else if (TargetFlags & AArch64II::MO_COFFSTUB) {
     Name = ".refptr.";
+  }
   Printer.TM.getNameWithPrefix(Name, GV,
                                Printer.getObjFileLowering().getMangler());
 
   MCSymbol *MCSym = Ctx.getOrCreateSymbol(Name);
 
   if (TargetFlags & AArch64II::MO_COFFSTUB) {
     MachineModuleInfoCOFF &MMICOFF =
         Printer.MMI->getObjFileInfo<MachineModuleInfoCOFF>();

llvm/lib/Target/AArch64/AArch64Subtarget.cpp

 AArch64Subtarget::ClassifyGlobalReference(const GlobalValue *GV,
                                           const TargetMachine &TM) const {
   // MachO large model always goes via a GOT, simply to get a single 8-byte
   // absolute relocation on all global addresses.
   if (TM.getCodeModel() == CodeModel::Large && isTargetMachO())
     return AArch64II::MO_GOT;
 
   if (!TM.shouldAssumeDSOLocal(*GV->getParent(), GV)) {
-    if (GV->hasDLLImportStorageClass())
+    if (GV->hasDLLImportStorageClass()) {
+      if (isWindowsArm64EC() && GV->getValueType()->isFunctionTy())
+        return AArch64II::MO_GOT | AArch64II::MO_DLLIMPORTAUX;
       return AArch64II::MO_GOT | AArch64II::MO_DLLIMPORT;
+    }
     if (getTargetTriple().isOSWindows())
       return AArch64II::MO_GOT | AArch64II::MO_COFFSTUB;
     return AArch64II::MO_GOT;
   }
 
   // The small code model's direct accesses use ADRP, which cannot
   // necessarily produce the value 0 (if the code is above 4GB).
   // Same for the tiny code model, where we have a pc relative LDR.
if (TM.getCodeModel() == CodeModel::Large && isTargetMachO() &&
     return AArch64II::MO_GOT;
 
   // NonLazyBind goes via GOT unless we know it's available locally.
   auto *F = dyn_cast<Function>(GV);
   if (UseNonLazyBind && F && F->hasFnAttribute(Attribute::NonLazyBind) &&
       !TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
     return AArch64II::MO_GOT;
 
+  if (getTargetTriple().isOSWindows()) {
+    if (isWindowsArm64EC() && GV->getValueType()->isFunctionTy() &&
+        GV->hasDLLImportStorageClass()) {
+      // On Arm64EC, if we're calling a function directly, use MO_DLLIMPORT,
+      // not MO_DLLIMPORTAUX.
+      return AArch64II::MO_GOT | AArch64II::MO_DLLIMPORT;
+    }
+
     // Use ClassifyGlobalReference for setting MO_DLLIMPORT/MO_COFFSTUB.
-  if (getTargetTriple().isOSWindows())
     return ClassifyGlobalReference(GV, TM);
+  }
 
   return AArch64II::MO_NO_FLAG;
 }
 
 void AArch64Subtarget::overrideSchedPolicy(MachineSchedPolicy &Policy,
                                            unsigned NumRegionInstrs) const {
   // LNT run (at least on Cyclone) showed reasonably significant gains for
   // bi-directional scheduling. 253.perlbmk.

llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h

enum TOF {
 
     /// MO_TAGGED - With MO_PAGE, indicates that the page includes a memory tag
     /// in bits 56-63.
     /// On a FrameIndex operand, indicates that the underlying memory is tagged
     /// with an unknown tag value (MTE); this needs to be lowered either to an
     /// SP-relative load or store instruction (which do not check tags), or to
     /// an LDG instruction to obtain the tag value.
     MO_TAGGED = 0x400,
+
+    /// MO_DLLIMPORTAUX - Symbol refers to "auxilliary" import stub. On
+    /// Arm64EC, there are two kinds of import stubs used for DLL import of
+    /// functions: MO_DLLIMPORT refers to natively callable Arm64 code, and
+    /// MO_DLLIMPORTAUX refers to the original address which can be compared
+    /// for equality.
+    MO_DLLIMPORTAUX = 0x800,
   };
 } // end namespace AArch64II
 
 namespace AArch64 {
 // The number of bits in a SVE register is architecturally defined
 // to be a multiple of this value.  If <M x t> has this number of bits,
 // a <n x M x t> vector can be stored in a SVE register without any
 // redundant bits.  If <M x t> has this number of bits divided by P,

llvm/test/CodeGen/AArch64/arm64ec-dllimport.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=arm64ec-pc-windows-msvc < %s | FileCheck %s
+
+@a = external dllimport global i32
+declare dllimport void @b()
+
+define ptr @dllimport_var() nounwind {
+; CHECK-LABEL: dllimport_var:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    adrp x0, __imp_a
+; CHECK-NEXT:    ldr x0, [x0, :lo12:__imp_a]
+; CHECK-NEXT:    ret
+  ret ptr @a
+}
+
+define ptr @dllimport_fn() nounwind {
+; CHECK-LABEL: dllimport_fn:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    .globl __imp_b
+; CHECK-NEXT:    adrp x0, __imp_aux_b
+; CHECK-NEXT:    .globl __imp_b
+; CHECK-NEXT:    ldr x0, [x0, :lo12:__imp_aux_b]
+; CHECK-NEXT:    ret
+  ret ptr @b
+}
+
+define void @dllimport_fn_call() nounwind {
+; CHECK-LABEL: dllimport_fn_call:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
+; CHECK-NEXT:    adrp x8, __imp_b
+; CHECK-NEXT:    ldr x8, [x8, :lo12:__imp_b]
+; CHECK-NEXT:    blr x8
+; CHECK-NEXT:    ldr x30, [sp], #16 // 8-byte Folded Reload
+; CHECK-NEXT:    ret
+  call void @b()
+  ret void
+}