[PATCH] D59709: [ThinLTO] Auto-hide prevailing linkonce_odr only when all copies eligible

Fri Mar 22 15:50:51 PDT 2019

pcc added a comment.

Does this has the correct behaviour in the case where the explicit instantiation is in a regular object file and the implicit instantiation is in bitcode?

In D59709#1440173 <https://reviews.llvm.org/D59709#1440173>, @steven_wu wrote:

> In D59709#1440113 <https://reviews.llvm.org/D59709#1440113>, @tejohnson wrote:
>
> > @pcc might have some info here. But I don't think so. We just can't mark hidden without the global visibility of all symbols, as we have here with ThinLTO (because of ELF picking the most hidden one).
>

ELF linkers don't do auto-hide because it would require extending ELF to store the auto-hide bit (or information from which it can be recovered) somewhere. We have extended ELF with an address-significance table which tells us whether symbols are `(local_)unnamed_addr`, but we aren't keeping track of whether symbols are `linkonce_odr`.

> If this is really linker semantics difference, would it be easier if just make the visibility hidden change in D43130 <https://reviews.llvm.org/D43130> be target specific?

That would mean losing the auto hide optimization on ELF. I don't think we should do that.

================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:202
+  /// Checks if all copies are eligible for auto-hiding (have flag set).
+  bool isCanAutoHide() const;
 };
----------------
`canAutoHide`? (same elsewhere)

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:984
+      if (NewLinkage == GlobalValue::WeakODRLinkage && GS->second->isCanAutoHide()) {
+        assert(GV.hasLinkOnceODRLinkage() && GV.hasGlobalUnnamedAddr());
         GV.setVisibility(GlobalValue::HiddenVisibility);
----------------
Can't it also be `local_unnamed_addr`?

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59709/new/

https://reviews.llvm.org/D59709