[PATCH] D51452: [MinGW] [AArch64] Add stubs for potential automatic dllimported variables

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/AArch64/AArch64Subtarget.cpp:204
+           !GV->isDSOLocal() && GV->isDeclarationForLinker() &&
+           isa<GlobalVariable>(GV))
+    Flags = AArch64II::MO_COFFSTUB | AArch64II::MO_GOT;
----------------
efriedma wrote:
> mstorsjo wrote:
> > efriedma wrote:
> > > 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.
> > 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.
> 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?
Oh, just read the discussion on D51288; refactoring the DSOLocal stuff incrementally should be fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D51452