Revert "[ThinLTO] Add an auto-hide feature"

This reverts commit r293970.

After more discussion, this belongs to the linker side and
there is no added value to do it at this level.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293993 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/IR/ModuleSummaryIndex.h

@@ -126,14 +126,11 @@ class GlobalValueSummary {
     /// 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 AutoHide)
+                     bool NotEligibleToImport, bool LiveRoot)
         : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport),
-          LiveRoot(LiveRoot), AutoHide(AutoHide) {}
+          LiveRoot(LiveRoot) {}
   };
 
 private:
@@ -201,9 +198,6 @@ class GlobalValueSummary {
     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, /* AutoHide */ false);
+                                          false);
       Elem.push_back(llvm::make_unique<FunctionSummary>(
           GVFlags, 0, ArrayRef<ValueInfo>{},
           ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests)));

include/llvm/LTO/LTO.h

@@ -53,18 +53,12 @@ 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<SummaryResolution(StringRef, GlobalValue::GUID)> isExported);
+    function_ref<bool(StringRef, GlobalValue::GUID)> isExported);
 
 namespace lto {
 

lib/Analysis/ModuleSummaryAnalysis.cpp

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

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -800,14 +800,13 @@ 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
-  bool NotEligibleToImport = ((RawFlags >> 4) & 0x1) || Version < 3;
+  RawFlags = RawFlags >> 4;
+  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 >> 5) & 0x1) || Version < 3;
-  bool AutoHide = (RawFlags >> 6) & 0x1;
-  return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot,
-                                     AutoHide);
+  bool LiveRoot = (RawFlags & 0x2) || Version < 3;
+  return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot);
 }
 
 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 |= Flags.Linkage;                    // 4 bits linkage
-  RawFlags |= (Flags.NotEligibleToImport << 4); // bool
-  RawFlags |= (Flags.LiveRoot << 5);            // bool
-  RawFlags |= (Flags.AutoHide << 6);            // bool
+  RawFlags = (RawFlags << 4) | Flags.Linkage; // 4 bits
+
   return RawFlags;
 }
 

lib/LTO/LTO.cpp

@@ -203,14 +203,11 @@ void llvm::thinLTOResolveWeakForLinkerInIndex(
 
 static void thinLTOInternalizeAndPromoteGUID(
     GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
-    function_ref<SummaryResolution(StringRef, GlobalValue::GUID)> isExported) {
+    function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &S : GVSummaryList) {
-    auto ExportResolution = isExported(S->modulePath(), GUID);
-    if (ExportResolution != Internal) {
+    if (isExported(S->modulePath(), GUID)) {
       if (GlobalValue::isLocalLinkage(S->linkage()))
         S->setLinkage(GlobalValue::ExternalLinkage);
-      if (ExportResolution == Hidden)
-        S->setAutoHide();
     } else if (!GlobalValue::isLocalLinkage(S->linkage()))
       S->setLinkage(GlobalValue::InternalLinkage);
   }
@@ -220,7 +217,7 @@ static void thinLTOInternalizeAndPromoteGUID(
 // as external and non-exported values as internal.
 void llvm::thinLTOInternalizeAndPromoteInIndex(
     ModuleSummaryIndex &Index,
-    function_ref<SummaryResolution(StringRef, GlobalValue::GUID)> isExported) {
+    function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &I : Index)
     thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported);
 }
@@ -954,20 +951,11 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                             const GlobalValueSummary *S) {
       return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath();
     };
-    auto isExported = [&](StringRef ModuleIdentifier,
-                          GlobalValue::GUID GUID) -> SummaryResolution {
+    auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
       const auto &ExportList = ExportLists.find(ModuleIdentifier);
-      if ((ExportList != ExportLists.end() && ExportList->second.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;
+      return (ExportList != ExportLists.end() &&
+              ExportList->second.count(GUID)) ||
+             ExportedGUIDs.count(GUID);
     };
     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>
-convertSymbolNamesToGUID(const StringSet<> &NamedSymbols,
-                         const Triple &TheTriple) {
-  DenseSet<GlobalValue::GUID> GUIDSymbols(NamedSymbols.size());
-  for (auto &Entry : NamedSymbols) {
+computeGUIDPreservedSymbols(const StringSet<> &PreservedSymbols,
+                            const Triple &TheTriple) {
+  DenseSet<GlobalValue::GUID> GUIDPreservedSymbols(PreservedSymbols.size());
+  for (auto &Entry : PreservedSymbols) {
     StringRef Name = Entry.first();
     if (TheTriple.isOSBinFormatMachO() && Name.size() > 0 && Name[0] == '_')
       Name = Name.drop_front();
-    GUIDSymbols.insert(GlobalValue::getGUID(Name));
+    GUIDPreservedSymbols.insert(GlobalValue::getGUID(Name));
   }
-  return GUIDSymbols;
+  return GUIDPreservedSymbols;
 }
 
 std::unique_ptr<MemoryBuffer> codegenModule(Module &TheModule,
@@ -554,7 +554,10 @@ void ThinLTOCodeGenerator::preserveSymbol(StringRef Name) {
 }
 
 void ThinLTOCodeGenerator::crossReferenceSymbol(StringRef Name) {
-  CrossReferencedSymbols.insert(Name);
+  // FIXME: At the moment, we don't take advantage of this extra information,
+  // we're conservatively considering cross-references as preserved.
+  //  CrossReferencedSymbols.insert(Name);
+  PreservedSymbols.insert(Name);
 }
 
 // TargetMachine factory
@@ -617,7 +620,7 @@ void ThinLTOCodeGenerator::promote(Module &TheModule,
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Convert the preserved symbols set from string to GUID
-  auto GUIDPreservedSymbols = convertSymbolNamesToGUID(
+  auto GUIDPreservedSymbols = computeGUIDPreservedSymbols(
       PreservedSymbols, Triple(TheModule.getTargetTriple()));
 
   // Compute "dead" symbols, we don't want to import/export these!
@@ -638,13 +641,11 @@ 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) -> SummaryResolution {
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) ||
-        GUIDPreservedSymbols.count(GUID))
-      return Exported;
-    return Internal;
+    return (ExportList != ExportLists.end() &&
+            ExportList->second.count(GUID)) ||
+           GUIDPreservedSymbols.count(GUID);
   };
   thinLTOInternalizeAndPromoteInIndex(Index, isExported);
 
@@ -664,7 +665,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
   Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Convert the preserved symbols set from string to GUID
-  auto GUIDPreservedSymbols = convertSymbolNamesToGUID(
+  auto GUIDPreservedSymbols = computeGUIDPreservedSymbols(
       PreservedSymbols, Triple(TheModule.getTargetTriple()));
 
   // Compute "dead" symbols, we don't want to import/export these!
@@ -738,7 +739,7 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
 
   // Convert the preserved symbols set from string to GUID
   auto GUIDPreservedSymbols =
-      convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple);
+      computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
@@ -760,13 +761,11 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule,
     return;
 
   // Internalization
-  auto isExported = [&](StringRef ModuleIdentifier,
-                        GlobalValue::GUID GUID) -> SummaryResolution {
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) ||
-        GUIDPreservedSymbols.count(GUID))
-      return Exported;
-    return Internal;
+    return (ExportList != ExportLists.end() &&
+            ExportList->second.count(GUID)) ||
+           GUIDPreservedSymbols.count(GUID);
   };
   thinLTOInternalizeAndPromoteInIndex(Index, isExported);
   thinLTOInternalizeModule(TheModule,
@@ -895,9 +894,7 @@ 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 =
-      convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple);
-  auto GUIDCrossRefSymbols =
-      convertSymbolNamesToGUID(CrossReferencedSymbols, TMBuilder.TheTriple);
+      computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
 
   // Compute "dead" symbols, we don't want to import/export these!
   auto DeadSymbols = computeDeadSymbols(*Index, GUIDPreservedSymbols);
@@ -919,16 +916,11 @@ void ThinLTOCodeGenerator::run() {
   // impacts the caching.
   resolveWeakForLinkerInIndex(*Index, ResolvedODR);
 
-  auto isExported = [&](StringRef ModuleIdentifier,
-                        GlobalValue::GUID GUID) -> SummaryResolution {
-    if (GUIDPreservedSymbols.count(GUID))
-      return Exported;
-    if (GUIDCrossRefSymbols.count(GUID))
-      return Hidden;
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
-    if (ExportList != ExportLists.end() && ExportList->second.count(GUID))
-      return Hidden;
-    return Internal;
+    return (ExportList != ExportLists.end() &&
+            ExportList->second.count(GUID)) ||
+           GUIDPreservedSymbols.count(GUID);
   };
 
   // Use global summary-based analysis to identify symbols that can be

lib/Transforms/IPO/FunctionImport.cpp

@@ -644,13 +644,6 @@ 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/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-LTO
+; 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=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-LTO2
+; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2
 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM
 
 ; Dead-stripping on the index allows to internalize these,
@@ -34,8 +34,7 @@
 
 ; Make sure we didn't internalize @boo, which is reachable via
 ; llvm.global_ctors
-; CHECK2-LTO: define void @boo()
-; CHECK2-LTO2: define hidden void @boo()
+; CHECK2: define void @boo()
 ; We should have eventually revoved @baz since it was internalized and unused
 ; CHECK2-NM-NOT: _baz