[ThinLTO] Add an auto-hide feature

When a symbol is not exported outside of the
DSO, it is can be hidden. Usually we try to internalize
as much as possible, but it is not always possible, for
instance a symbol can be referenced outside of the LTO
unit, or there can be cross-module reference in ThinLTO.

This is a recommit of r293912 after fixing build failures.

Differential Revision: https://reviews.llvm.org/D28978

llvm-svn: 293918

include/llvm/IR/ModuleSummaryIndex.h

@@ -126,11 +126,14 @@ public:
     /// llvm.global_ctors that the linker does not know about.
     unsigned LiveRoot : 1;
 
+    /// Indicate if the global value should be hidden.
+    unsigned AutoHide : 1;
+
     /// Convenience Constructors
     explicit GVFlags(GlobalValue::LinkageTypes Linkage,
-                     bool NotEligibleToImport, bool LiveRoot)
+                     bool NotEligibleToImport, bool LiveRoot, bool AutoHide)
         : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport),
-          LiveRoot(LiveRoot) {}
+          LiveRoot(LiveRoot), AutoHide(AutoHide) {}
   };
 
 private:
@@ -198,6 +201,9 @@ public:
     Flags.Linkage = Linkage;
   }
 
+  /// Sets the visibility to be autohidden.
+  void setAutoHide() { Flags.AutoHide = true; }
+
   /// Return true if this global value can't be imported.
   bool notEligibleToImport() const { return Flags.NotEligibleToImport; }
 

include/llvm/IR/ModuleSummaryIndexYAML.h

@@ -79,7 +79,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
     auto &Elem = V[KeyInt];
     for (auto &FSum : FSums) {
       GlobalValueSummary::GVFlags GVFlags(GlobalValue::ExternalLinkage, false,
-                                          false);
+                                          false, /* AutoHide */ false);
       Elem.push_back(llvm::make_unique<FunctionSummary>(
           GVFlags, 0, ArrayRef<ValueInfo>{},
           ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests)));

include/llvm/LTO/LTO.h

@@ -52,12 +52,18 @@ void thinLTOResolveWeakForLinkerInIndex(
     function_ref<void(StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes)>
         recordNewLinkage);
 
+/// This enum is used for the returned value of the callback passed to
+/// thinLTOInternalizeAndPromoteInIndex, it indicates if a symbol can be made
+/// Internal (only referenced from its defining object), Hidden (
+/// outside the DSO), or Exported (exposed as public API for the DSO).
+enum SummaryResolution { Internal, Hidden, Exported };
+
 /// Update the linkages in the given \p Index to mark exported values
 /// as external and non-exported values as internal. The ThinLTO backends
 /// must apply the changes to the Module via thinLTOInternalizeModule.
 void thinLTOInternalizeAndPromoteInIndex(
     ModuleSummaryIndex &Index,
-    function_ref<bool(StringRef, GlobalValue::GUID)> isExported);
+    function_ref<SummaryResolution(StringRef, GlobalValue::GUID)> isExported);
 
 namespace lto {
 

lib/Analysis/ModuleSummaryAnalysis.cpp

@@ -190,7 +190,8 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
       // FIXME: refactor this to use the same code that inliner is using.
       F.isVarArg();
   GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
-                                    /* LiveRoot = */ false);
+                                    /* LiveRoot = */ false,
+                                    /* AutoHide */ false);
   auto FuncSummary = llvm::make_unique<FunctionSummary>(
       Flags, NumInsts, RefEdges.takeVector(), CallGraphEdges.takeVector(),
       TypeTests.takeVector());
@@ -207,7 +208,8 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
   findRefEdges(&V, RefEdges, Visited);
   bool NonRenamableLocal = isNonRenamableLocal(V);
   GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal,
-                                    /* LiveRoot = */ false);
+                                    /* LiveRoot = */ false,
+                                    /* AutoHide */ false);
   auto GVarSummary =
       llvm::make_unique<GlobalVarSummary>(Flags, RefEdges.takeVector());
   if (NonRenamableLocal)
@@ -220,7 +222,8 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
                     DenseSet<GlobalValue::GUID> &CantBePromoted) {
   bool NonRenamableLocal = isNonRenamableLocal(A);
   GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal,
-                                    /* LiveRoot = */ false);
+                                    /* LiveRoot = */ false,
+                                    /* AutoHide */ false);
   auto AS = llvm::make_unique<AliasSummary>(Flags, ArrayRef<ValueInfo>{});
   auto *Aliasee = A.getBaseObject();
   auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee);
@@ -339,7 +342,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
           assert(GV->isDeclaration() && "Def in module asm already has definition");
           GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage,
                                               /* NotEligibleToImport */ true,
-                                              /* LiveRoot */ true);
+                                              /* LiveRoot */ true,
+                                              /* AutoHide */ false);
           CantBePromoted.insert(GlobalValue::getGUID(Name));
           // Create the appropriate summary type.
           if (isa<Function>(GV)) {

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -800,13 +800,14 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
   // like getDecodedLinkage() above. Any future change to the linkage enum and
   // to getDecodedLinkage() will need to be taken into account here as above.
   auto Linkage = GlobalValue::LinkageTypes(RawFlags & 0xF); // 4 bits
-  RawFlags = RawFlags >> 4;
+  bool NotEligibleToImport = ((RawFlags >> 4) & 0x1) || Version < 3;
-  bool NotEligibleToImport = (RawFlags & 0x1) || Version < 3;
   // The LiveRoot flag wasn't introduced until version 3. For dead stripping
   // to work correctly on earlier versions, we must conservatively treat all
   // values as live.
-  bool LiveRoot = (RawFlags & 0x2) || Version < 3;
+  bool LiveRoot = ((RawFlags >> 5) & 0x1) || Version < 3;
-  return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot);
+  bool AutoHide = (RawFlags >> 6) & 0x1;
+  return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot,
+                                     AutoHide);
 }
 
 static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {

lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -971,13 +971,13 @@ static unsigned getEncodedLinkage(const GlobalValue &GV) {
 static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
   uint64_t RawFlags = 0;
 
-  RawFlags |= Flags.NotEligibleToImport; // bool
-  RawFlags |= (Flags.LiveRoot << 1);
   // Linkage don't need to be remapped at that time for the summary. Any future
   // change to the getEncodedLinkage() function will need to be taken into
   // account here as well.
-  RawFlags = (RawFlags << 4) | Flags.Linkage; // 4 bits
+  RawFlags |= Flags.Linkage;                    // 4 bits linkage
-
+  RawFlags |= (Flags.NotEligibleToImport << 4); // bool
+  RawFlags |= (Flags.LiveRoot << 5);            // bool
+  RawFlags |= (Flags.AutoHide << 6);            // bool
   return RawFlags;
 }
 

lib/LTO/LTO.cpp

@@ -199,11 +199,14 @@ void llvm::thinLTOResolveWeakForLinkerInIndex(
 
 static void thinLTOInternalizeAndPromoteGUID(
     GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
-    function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
+    function_ref<SummaryResolution(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &S : GVSummaryList) {
-    if (isExported(S->modulePath(), GUID)) {
+    auto ExportResolution = isExported(S->modulePath(), GUID);
+    if (ExportResolution != Internal) {
       if (GlobalValue::isLocalLinkage(S->linkage()))
         S->setLinkage(GlobalValue::ExternalLinkage);
+      if (ExportResolution == Hidden)
+        S->setAutoHide();
     } else if (!GlobalValue::isLocalLinkage(S->linkage()))
       S->setLinkage(GlobalValue::InternalLinkage);
   }
@@ -213,7 +216,7 @@ static void thinLTOInternalizeAndPromoteGUID(
 // as external and non-exported values as internal.
 void llvm::thinLTOInternalizeAndPromoteInIndex(
     ModuleSummaryIndex &Index,
-    function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
+    function_ref<SummaryResolution(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &I : Index)
     thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported);
 }
@@ -921,11 +924,20 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                             const GlobalValueSummary *S) {
       return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath();
     };
-    auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    auto isExported = [&](StringRef ModuleIdentifier,
+                          GlobalValue::GUID GUID) -> SummaryResolution {
       const auto &ExportList = ExportLists.find(ModuleIdentifier);
-      return (ExportList != ExportLists.end() &&
+      if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) ||
-              ExportList->second.count(GUID)) ||
+          ExportedGUIDs.count(GUID)) {
-             ExportedGUIDs.count(GUID);
+        // We could do better by hiding when a symbol is in
+        // GUIDPreservedSymbols because it is only referenced from regular LTO
+        // or from native files and not outside the final binary, but that's
+        // something the native linker could do as gwell.
+        if (GUIDPreservedSymbols.count(GUID))
+          return Exported;
+        return Hidden;
+      }
+      return Internal;
     };
     thinLTOInternalizeAndPromoteInIndex(ThinLTO.CombinedIndex, isExported);
 

lib/LTO/ThinLTOCodeGenerator.cpp

@@ -234,16 +234,16 @@ static void optimizeModule(Module &TheModule, TargetMachine &TM,
 
 // Convert the PreservedSymbols map from "Name" based to "GUID" based.
 static DenseSet<GlobalValue::GUID>
-computeGUIDPreservedSymbols(const StringSet<> &PreservedSymbols,
+convertSymbolNamesToGUID(const StringSet<> &NamedSymbols,
-                            const Triple &TheTriple) {
+                         const Triple &TheTriple) {
-  DenseSet<GlobalValue::GUID> GUIDPreservedSymbols(PreservedSymbols.size());
+  DenseSet<GlobalValue::GUID> GUIDSymbols(NamedSymbols.size());
-  for (auto &Entry : PreservedSymbols) {
+  for (auto &Entry : NamedSymbols) {
     StringRef Name = Entry.first();
     if (TheTriple.isOSBinFormatMachO() && Name.size() > 0 && Name[0] == '_')
       Name = Name.drop_front();
-    GUIDPreservedSymbols.insert(GlobalValue::getGUID(Name));
+    GUIDSymbols.insert(GlobalValue::getGUID(Name));
   }
-  return GUIDPreservedSymbols;
+  return GUIDSymbols;
 }
 
 std::unique_ptr<MemoryBuffer> codegenModule(Module &TheModule,
@@ -554,10 +554,7 @@ void ThinLTOCodeGenerator::preserveSymbol(StringRef Name) {
 }
 
 void ThinLTOCodeGenerator::crossReferenceSymbol(StringRef Name) {
-  // FIXME: At the moment, we don't take advantage of this extra information,
+  CrossReferencedSymbols.insert(Name);
-  // we're conservatively considering cross-references as preserved.
-  //  CrossReferencedSymbols.insert(Name);
-  PreservedSymbols.insert(Name);
 }
 
 // TargetMachine factory
@@ -620,7 +617,7 @@ void ThinLTOCodeGenerator::promote(Module &TheModule,
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Convert the preserved symbols set from string to GUID
-  auto GUIDPreservedSymbols = computeGUIDPreservedSymbols(
+  auto GUIDPreservedSymbols = convertSymbolNamesToGUID(
       PreservedSymbols, Triple(TheModule.getTargetTriple()));
 
   // Compute "dead" symbols, we don't want to import/export these!
@@ -641,11 +638,13 @@ void ThinLTOCodeGenerator::promote(Module &TheModule,
 
   // Promote the exported values in the index, so that they are promoted
   // in the module.
-  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+  auto isExported = [&](StringRef ModuleIdentifier,
+                        GlobalValue::GUID GUID) -> SummaryResolution {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    return (ExportList != ExportLists.end() &&
+    if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) ||
-            ExportList->second.count(GUID)) ||
+        GUIDPreservedSymbols.count(GUID))
-           GUIDPreservedSymbols.count(GUID);
+      return Exported;
+    return Internal;
   };
   thinLTOInternalizeAndPromoteInIndex(Index, isExported);
 
@@ -665,7 +664,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Convert the preserved symbols set from string to GUID
-  auto GUIDPreservedSymbols = computeGUIDPreservedSymbols(
+  auto GUIDPreservedSymbols = convertSymbolNamesToGUID(
       PreservedSymbols, Triple(TheModule.getTargetTriple()));
 
   // Compute "dead" symbols, we don't want to import/export these!
@@ -739,7 +738,7 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
 
   // Convert the preserved symbols set from string to GUID
   auto GUIDPreservedSymbols =
-      computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
+      convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple);
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
@@ -761,11 +760,13 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
     return;
 
   // Internalization
-  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+  auto isExported = [&](StringRef ModuleIdentifier,
+                        GlobalValue::GUID GUID) -> SummaryResolution {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    return (ExportList != ExportLists.end() &&
+    if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) ||
-            ExportList->second.count(GUID)) ||
+        GUIDPreservedSymbols.count(GUID))
-           GUIDPreservedSymbols.count(GUID);
+      return Exported;
+    return Internal;
   };
   thinLTOInternalizeAndPromoteInIndex(Index, isExported);
   thinLTOInternalizeModule(TheModule,
@@ -894,7 +895,9 @@ void ThinLTOCodeGenerator::run() {
   // Convert the preserved symbols set from string to GUID, this is needed for
   // computing the caching hash and the internalization.
   auto GUIDPreservedSymbols =
-      computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
+      convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple);
+  auto GUIDCrossRefSymbols =
+      convertSymbolNamesToGUID(CrossReferencedSymbols, TMBuilder.TheTriple);
 
   // Compute "dead" symbols, we don't want to import/export these!
   auto DeadSymbols = computeDeadSymbols(*Index, GUIDPreservedSymbols);
@@ -916,11 +919,16 @@ void ThinLTOCodeGenerator::run() {
   // impacts the caching.
   resolveWeakForLinkerInIndex(*Index, ResolvedODR);
 
-  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+  auto isExported = [&](StringRef ModuleIdentifier,
+                        GlobalValue::GUID GUID) -> SummaryResolution {
+    if (GUIDPreservedSymbols.count(GUID))
+      return Exported;
+    if (GUIDCrossRefSymbols.count(GUID))
+      return Hidden;
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    return (ExportList != ExportLists.end() &&
+    if (ExportList != ExportLists.end() && ExportList->second.count(GUID))
-            ExportList->second.count(GUID)) ||
+      return Hidden;
-           GUIDPreservedSymbols.count(GUID);
+    return Internal;
   };
 
   // Use global summary-based analysis to identify symbols that can be

lib/Transforms/IPO/FunctionImport.cpp

@@ -650,6 +650,13 @@ void llvm::thinLTOInternalizeModule(Module &TheModule,
     return !GlobalValue::isLocalLinkage(Linkage);
   };
 
+  // Try to auto-hide the symbols.
+  for (auto &GO : TheModule.global_objects()) {
+    const auto &GS = DefinedGlobals.find(GO.getGUID());
+    if (GS != DefinedGlobals.end() && GS->second->flags().AutoHide)
+      GO.setVisibility(GlobalValue::HiddenVisibility);
+  }
+
   // FIXME: See if we can just internalize directly here via linkage changes
   // based on the index, rather than invoking internalizeModule.
   llvm::internalizeModule(TheModule, MustPreserveGV);

test/ThinLTO/X86/Inputs/weak_autohide.ll

@@ -0,0 +1,6 @@
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+define weak_odr void @weakodrfunc() {
+  ret void
+}

test/ThinLTO/X86/deadstrip.ll

@@ -3,7 +3,7 @@
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 
 ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t1.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t1.bc - -o - | llvm-dis -o - | FileCheck %s
-; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t2.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK2
+; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t2.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK2-LTO
 
 ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=run %t1.bc %t2.bc
 ; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=CHECK-NM
@@ -19,7 +19,7 @@
 ; RUN:   -r %t2.bc,_dead_func,pl \
 ; RUN:   -r %t2.bc,_another_dead_func,pl
 ; RUN: llvm-dis < %t.out.0.3.import.bc | FileCheck %s
-; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2
+; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2-LTO2
 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM
 
 ; Dead-stripping on the index allows to internalize these,
@@ -34,7 +34,8 @@
 
 ; Make sure we didn't internalize @boo, which is reachable via
 ; llvm.global_ctors
-; CHECK2: define void @boo()
+; CHECK2-LTO: define void @boo()
+; CHECK2-LTO2: define hidden void @boo()
 ; We should have eventually revoved @baz since it was internalized and unused
 ; CHECK2-NM-NOT: _baz
 

test/ThinLTO/X86/weak_autohide.ll

@@ -0,0 +1,24 @@
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/weak_autohide.ll -o %t2.bc
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,_strong_func,pxl \
+; RUN:     -r=%t1.bc,_weakodrfunc,pl \
+; RUN:     -r=%t2.bc,_weakodrfunc,l
+; RUN: llvm-dis < %t.o.0.2.internalize.bc | FileCheck  %s --check-prefix=AUTOHIDE
+
+
+; AUTOHIDE: weak_odr hidden void @weakodrfunc
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+define weak_odr void @weakodrfunc() #0 {
+  ret void
+}
+
+define void @strong_func() #0 {
+	call void @weakodrfunc()
+	ret void
+}
+