[PATCH] D73228: [AsmPrinter][ELF] Define local aliases (.Lfoo$local) for GlobalObjects
Sean Fertile via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 29 08:35:12 PST 2020
sfertile accepted this revision.
sfertile added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:456
+ if (TM.getTargetTriple().isOSBinFormatELF() &&
+ GlobalObject::isExternalLinkage(GV.getLinkage()) && GV.isDSOLocal() &&
+ !GV.isDeclaration() && isa<GlobalObject>(GV))
----------------
MaskRay wrote:
> sfertile wrote:
> > Ideally `GV.isDSOLocal() ` should fully determine if a symbol is replaceable at runtime or not. Not sure if its safer to use `TargetMachine::shouldAssumeDSOLocal` instead. I believe the only difference right now for ELF would be the copy relocation case.
> >
> > That said, this conditional is only true if the symbol is not preemptable. IIUC the compiler shouldn't be accessing data GOT-indirect, and the linker won't be emitting PLTs for the functions. Which means:
> >
> > ```
> > costs brought by potential symbol interpositionloc
> > (relocations in object files, PLT/GOT in executables/shared objects).
> > ```
> >
> > There shouldn't be any saving related to PLT/GOT. I'm not very familiar with linking on archs other then Power though so I could definitely be wrong here.
> It is not convenient to access `Module` in this function. We probably should improve `GlobalVariable::maybeSetDsoLocal` to take account of `TargetMachine::shouldAssumeDSOLocal`, so that isDSOLocal() and `TargetMachine::shouldAssumeDSOLocal` will return the same value.
>
> We require a definition (`!GV.isDeclaration()`). So for copy relocation, e.g.
>
> ```
> // x86 x86-64
> // or aarch64 -mpie-copy-relocations
> extern int var;
> int foo() { return var; }
> ```
>
> We don't use a local alias (no issue here).
Good point, thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73228/new/
https://reviews.llvm.org/D73228
More information about the llvm-commits
mailing list