[MinGW] [AArch64] Add stubs for potential automatic dllimported variables

[mstorsjo]

Summary

The runtime pseudo relocations can't handle the AArch64 format PC relative addressing in adrp+add/ldr pairs. By using stubs, the potentially dllimported addresses can be touched up by the runtime pseudo relocation framework.

The MachineOperand TargetFlags range (8 bits) is just about exhausted; this allocates the last bit to MO_COFFSTUB. (In the X86 target, the TargetFlags are just direct values, while most of it is used as a bitfield on ARM and AArch64.) An alternative, to conserve bits, would be to use MO_GOT (which doesn't have much meaning in a COFF context right now).

Currently in the AArch64 target, the MO_GOT flag is set for any indirect access (so we have MO_GOT | MO_DLLIMPORT or MO_GOT | MO_COFFSTUB).

[efriedma]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

I don't see why it matters whether the value is a GlobalVariable, specifically, as opposed to an alias or function.

I don't see why this applies specifically to MinGW, as opposed to all Windows targets.

I'm not sure what the point of the isDeclarationForLinker check is... it seems redundant with the isDSOLocal check.

[mstorsjo]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

Sorry, there's probably not nearly enough context here - this is the AArch64 followup to D51382 and D51288; it should do the same as the latter but for the AArch64 target.

This relates to MinGW and automatically importing data variables from another DLL, even though the code referencing the variable didn't use a dllimport attribute.

For functions, in case they turn out to need to be imported from a DLL even though the access didn't know that, the linker will just insert a thunk and use that instead (and even MSVC supports this).

To handle the same for data, though, the linker must create a list of deferred relocations that the MinGW runtime will process on load, to fix things up for cases when a variable turned out to be automatically imported from a DLL. All of this is a MinGW specific concept; MSVC's runtime and linker don't support it.

This patch reroutes accesses to variables that potentially might need to be imported from a DLL via a stub pointer, to avoid having the runtime fixup touch the code section.

As for aliases, I'm not familiar enough with them on the IR level to know whether they should get the same treatment or not.

[efriedma]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

Okay, that makes sense.

For the alias thing, I guess the right way to deal with it is to check something like !isa<FunctionType>(GV->getValueType()).

Still not sure why you need the isDeclarationForLinker check, given you already check isDSOLocal(). Does the isDSOLocal() check not work correctly in some cases?

[efriedma]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

Oh, just read the discussion on D51288; refactoring the DSOLocal stuff incrementally should be fine.

[mstorsjo]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

Ok, checking for functions like that might be good. We have a bunch of similar checks added in both of the earlier commits, and since I think that's a fringe case (and I'm planning to refactor parts of this to reduce the duplication, once all of X86, ARM and AArch64 supports this) perhaps it makes sense to postpone that change until then?

As for isDeclarationForLinker(), originally I had hasExternalLinkage() && isDeclaration() (with the intent to only do this for variables where we don't see any definition within the current scope). In D51382 (in clang, to skip setting the dso_local for potentially imported variables), @rnk suggested to use isDeclarationForLinker(). Even though the distinction between the two might not make sense at this spot (while it does when generating IR in clang), I think it makes sense to try to keep the (currently quite scattered) conditions as similar as possible.

[mstorsjo]

@rnk - FWIW, this one should be pretty much similar to the ARM one that already was approved and merged. I'll update the second value to StubValueTy just like in the other one as well before committing; I'll do the generic refactoring and deduplication between X86/ARM/AArch64 when this one is done, and then we can consider tweaking the condition as suggested by @efriedma.

[efriedma]

Okay.

[mstorsjo]

Updated the patch after SVN r341310 where I cleaned up and simplified some related code.

[mstorsjo]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

For the alias thing, I guess the right way to deal with it is to check something like !isa<FunctionType>(GV->getValueType()).

I started looking at this part now (for a separate later commit), but is it at all possible to declare an external alias? I don't find anything relevant under test/CodeGen at least.

As soon as we see a definition of the variable, we know that the variable really will be available in the local DSO, and we bypass the stub altogether. So this is only invoked for variables that are declared but not defined.

[efriedma]

lib/Target/AArch64/AArch64Subtarget.cpp

204 ↗

(On Diff #163159)

You're right, an alias always points to a variable or function definition, so that isn't relevant.

[mstorsjo]

Is anyone up for approving this one (@efriedma @rnk) now then? The ARM one was already approved and committed last week.

[rnk]

lgtm

Now I see the relation to D51590.

llvm/trunk/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 (GV->hasDLLImportStorageClass())
+  if (OpFlags & (AArch64II::MO_DLLIMPORT | 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/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp

bool AArch64InstrInfo::expandPostRAPseudo(MachineInstr &MI) const {
   const GlobalValue *GV =
       cast<GlobalValue>((*MI.memoperands_begin())->getValue());
   const TargetMachine &TM = MBB.getParent()->getTarget();
   unsigned char OpFlags = Subtarget.ClassifyGlobalReference(GV, TM);
   const unsigned char MO_NC = AArch64II::MO_NC;
 
   if ((OpFlags & AArch64II::MO_GOT) != 0) {
     BuildMI(MBB, MI, DL, get(AArch64::LOADgot), Reg)
-        .addGlobalAddress(GV, 0, AArch64II::MO_GOT);
+        .addGlobalAddress(GV, 0, OpFlags);
     BuildMI(MBB, MI, DL, get(AArch64::LDRXui), Reg)
         .addReg(Reg, RegState::Kill)
         .addImm(0)
         .addMemOperand(*MI.memoperands_begin());
   } else if (TM.getCodeModel() == CodeModel::Large) {
     BuildMI(MBB, MI, DL, get(AArch64::MOVZXi), Reg)
         .addGlobalAddress(GV, 0, AArch64II::MO_G0 | MO_NC)
         .addImm(0);
AArch64InstrInfo::getSerializableDirectMachineOperandTargetFlags() const {
   return makeArrayRef(TargetFlags);
 }
 
 ArrayRef<std::pair<unsigned, const char *>>
 AArch64InstrInfo::getSerializableBitmaskMachineOperandTargetFlags() const {
   using namespace AArch64II;
 
   static const std::pair<unsigned, const char *> TargetFlags[] = {
+      {MO_COFFSTUB, "aarch64-coffstub"},
       {MO_GOT, "aarch64-got"},   {MO_NC, "aarch64-nc"},
       {MO_TLS, "aarch64-tls"},   {MO_DLLIMPORT, "aarch64-dllimport"}};
   return makeArrayRef(TargetFlags);
 }
 
 ArrayRef<std::pair<MachineMemOperand::Flags, const char *>>
 AArch64InstrInfo::getSerializableMachineMemOperandTargetFlags() const {
   static const std::pair<MachineMemOperand::Flags, const char *> TargetFlags[] =

llvm/trunk/lib/Target/AArch64/AArch64MCInstLower.cpp

 //===----------------------------------------------------------------------===//
 
 #include "AArch64MCInstLower.h"
 #include "MCTargetDesc/AArch64MCExpr.h"
 #include "Utils/AArch64BaseInfo.h"
 #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/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
 #include "llvm/Target/TargetMachine.h"
AArch64MCInstLower::GetGlobalAddressSymbol(const MachineOperand &MO) const {
   unsigned TargetFlags = MO.getTargetFlags();
   const Triple &TheTriple = Printer.TM.getTargetTriple();
   if (!TheTriple.isOSBinFormatCOFF())
     return Printer.getSymbol(GV);
 
   assert(TheTriple.isOSWindows() &&
          "Windows is the only supported COFF target");
 
-  bool IsIndirect = (TargetFlags & AArch64II::MO_DLLIMPORT);
+  bool IsIndirect = (TargetFlags & (AArch64II::MO_DLLIMPORT | AArch64II::MO_COFFSTUB));
   if (!IsIndirect)
     return Printer.getSymbol(GV);
 
   SmallString<128> Name;
+  if (TargetFlags & AArch64II::MO_DLLIMPORT)
     Name = "__imp_";
+  else if (TargetFlags & AArch64II::MO_COFFSTUB)
+    Name = ".refptr.";
   Printer.TM.getNameWithPrefix(Name, GV,
                                Printer.getObjFileLowering().getMangler());
 
-  return Ctx.getOrCreateSymbol(Name);
+  MCSymbol *MCSym = Ctx.getOrCreateSymbol(Name);
+
+  if (TargetFlags & AArch64II::MO_COFFSTUB) {
+    MachineModuleInfoCOFF &MMICOFF =
+        Printer.MMI->getObjFileInfo<MachineModuleInfoCOFF>();
+    MachineModuleInfoImpl::StubValueTy &StubSym =
+        MMICOFF.getGVStubEntry(MCSym);
+
+    if (!StubSym.getPointer())
+      StubSym = MachineModuleInfoImpl::StubValueTy(Printer.getSymbol(GV), true);
+  }
+
+  return MCSym;
 }
 
 MCSymbol *
 AArch64MCInstLower::GetExternalSymbolSymbol(const MachineOperand &MO) const {
   return Printer.GetExternalSymbolSymbol(MO.getSymbolName());
 }
 
 MCOperand AArch64MCInstLower::lowerSymbolOperandDarwin(const MachineOperand &MO,

llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp

 unsigned char
 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;
 
-  unsigned Flags = GV->hasDLLImportStorageClass() ? AArch64II::MO_DLLIMPORT
-                                                  : AArch64II::MO_NO_FLAG;
+  unsigned Flags = AArch64II::MO_NO_FLAG;
+  if (GV->hasDLLImportStorageClass())
+    Flags = AArch64II::MO_DLLIMPORT;
+  else if (getTargetTriple().isWindowsGNUEnvironment() &&
+           !GV->isDSOLocal() && GV->isDeclarationForLinker() &&
+           isa<GlobalVariable>(GV))
+    Flags = AArch64II::MO_COFFSTUB | AArch64II::MO_GOT;
 
   if (!TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
     return AArch64II::MO_GOT | Flags;
 
   // 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 ((useSmallAddressing() || TM.getCodeModel() == CodeModel::Tiny) &&

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

 namespace AArch64II {
   /// Target Operand Flag enum.
   enum TOF {
     //===------------------------------------------------------------------===//
     // AArch64 Specific MachineOperand flags.
 
     MO_NO_FLAG,
 
-    MO_FRAGMENT = 0xf,
+    MO_FRAGMENT = 0x7,
 
     /// MO_PAGE - A symbol operand with this flag represents the pc-relative
     /// offset of the 4K page containing the symbol.  This is used with the
     /// ADRP instruction.
     MO_PAGE = 1,
 
     /// MO_PAGEOFF - A symbol operand with this flag represents the offset of
     /// that symbol within a 4K page.  This offset is added to the page address
enum TOF {
     /// 0-15 of a 64-bit address, used in a MOVZ or MOVK instruction
     MO_G0 = 6,
 
     /// MO_HI12 - This flag indicates that a symbol operand represents the bits
     /// 13-24 of a 64-bit address, used in a arithmetic immediate-shifted-left-
     /// by-12-bits instruction.
     MO_HI12 = 7,
 
+    /// MO_COFFSTUB - On a symbol operand "FOO", this indicates that the
+    /// reference is actually to the ".refptrp.FOO" symbol.  This is used for
+    /// stub symbols on windows.
+    MO_COFFSTUB = 0x8,
+
     /// MO_GOT - This flag indicates that a symbol operand represents the
     /// address of the GOT entry for the symbol, rather than the address of
     /// the symbol itself.
     MO_GOT = 0x10,
 
     /// MO_NC - Indicates whether the linker is expected to check the symbol
     /// reference for overflow. For example in an ADRP/ADD pair of relocations
     /// the ADRP usually does check, but not the ADD.

llvm/trunk/test/CodeGen/AArch64/mingw-refptr.ll

+; RUN: llc < %s -mtriple=aarch64-w64-mingw32 | FileCheck %s
+
+@var = external local_unnamed_addr global i32, align 4
+@dsolocalvar = external dso_local local_unnamed_addr global i32, align 4
+@localvar = dso_local local_unnamed_addr global i32 0, align 4
+@localcommon = common dso_local local_unnamed_addr global i32 0, align 4
+@extvar = external dllimport local_unnamed_addr global i32, align 4
+
+define dso_local i32 @getVar() {
+; CHECK-LABEL: getVar:
+; CHECK:    adrp x8, .refptr.var
+; CHECK:    ldr  x8, [x8, .refptr.var]
+; CHECK:    ldr  w0, [x8]
+; CHECK:    ret
+entry:
+  %0 = load i32, i32* @var, align 4
+  ret i32 %0
+}
+
+define dso_local i32 @getDsoLocalVar() {
+; CHECK-LABEL: getDsoLocalVar:
+; CHECK:    adrp x8, dsolocalvar
+; CHECK:    ldr  w0, [x8, dsolocalvar]
+; CHECK:    ret
+entry:
+  %0 = load i32, i32* @dsolocalvar, align 4
+  ret i32 %0
+}
+
+define dso_local i32 @getLocalVar() {
+; CHECK-LABEL: getLocalVar:
+; CHECK:    adrp x8, localvar
+; CHECK:    ldr  w0, [x8, localvar]
+; CHECK:    ret
+entry:
+  %0 = load i32, i32* @localvar, align 4
+  ret i32 %0
+}
+
+define dso_local i32 @getLocalCommon() {
+; CHECK-LABEL: getLocalCommon:
+; CHECK:    adrp x8, localcommon
+; CHECK:    ldr  w0, [x8, localcommon]
+; CHECK:    ret
+entry:
+  %0 = load i32, i32* @localcommon, align 4
+  ret i32 %0
+}
+
+define dso_local i32 @getExtVar() {
+; CHECK-LABEL: getExtVar:
+; CHECK:    adrp x8, __imp_extvar
+; CHECK:    ldr  x8, [x8, __imp_extvar]
+; CHECK:    ldr  w0, [x8]
+; CHECK:    ret
+entry:
+  %0 = load i32, i32* @extvar, align 4
+  ret i32 %0
+}
+
+define dso_local void @callFunc() {
+; CHECK-LABEL: callFunc:
+; CHECK:    b otherFunc
+entry:
+  tail call void @otherFunc()
+  ret void
+}
+
+declare dso_local void @otherFunc()
+
+define dso_local void @sspFunc() #0 {
+; CHECK-LABEL: sspFunc:
+; CHECK:    adrp x8, .refptr.__stack_chk_guard
+; CHECK:    ldr  x8, [x8, .refptr.__stack_chk_guard]
+; CHECK:    ldr  x8, [x8]
+entry:
+  %c = alloca i8, align 1
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* nonnull %c)
+  call void @ptrUser(i8* nonnull %c)
+  call void @llvm.lifetime.end.p0i8(i64 1, i8* nonnull %c)
+  ret void
+}
+
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
+declare dso_local void @ptrUser(i8*) local_unnamed_addr #2
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
+
+attributes #0 = { sspstrong }
+
+; CHECK:        .section        .rdata$.refptr.__stack_chk_guard,"dr",discard,.refptr.__stack_chk_guard
+; CHECK:        .globl  .refptr.__stack_chk_guard
+; CHECK: .refptr.__stack_chk_guard:
+; CHECK:        .xword  __stack_chk_guard
+; CHECK:        .section        .rdata$.refptr.var,"dr",discard,.refptr.var
+; CHECK:        .globl  .refptr.var
+; CHECK: .refptr.var:
+; CHECK:        .xword  var