]> granicus.if.org Git - llvm/commitdiff
Revert "[ThinLTO] Add an auto-hide feature"
authorMehdi Amini <mehdi.amini@apple.com>
Thu, 2 Feb 2017 18:24:37 +0000 (18:24 +0000)
committerMehdi Amini <mehdi.amini@apple.com>
Thu, 2 Feb 2017 18:24:37 +0000 (18:24 +0000)
This reverts r293912, bots are broken.

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

12 files changed:
include/llvm/IR/ModuleSummaryIndex.h
include/llvm/IR/ModuleSummaryIndexYAML.h
include/llvm/LTO/LTO.h
lib/Analysis/ModuleSummaryAnalysis.cpp
lib/Bitcode/Reader/BitcodeReader.cpp
lib/Bitcode/Writer/BitcodeWriter.cpp
lib/LTO/LTO.cpp
lib/LTO/ThinLTOCodeGenerator.cpp
lib/Transforms/IPO/FunctionImport.cpp
test/ThinLTO/X86/Inputs/weak_autohide.ll [deleted file]
test/ThinLTO/X86/deadstrip.ll
test/ThinLTO/X86/weak_autohide.ll [deleted file]

index 967b5c7a85973239c22197fb5bcaafdfa214cba3..c710c41cccd08cbd716cbe0d035bb2bacadddcd1 100644 (file)
@@ -126,14 +126,11 @@ 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 AutoHide)
+                     bool NotEligibleToImport, bool LiveRoot)
         : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport),
-          LiveRoot(LiveRoot), AutoHide(AutoHide) {}
+          LiveRoot(LiveRoot) {}
   };
 
 private:
@@ -201,9 +198,6 @@ 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; }
 
index c880abfea2456b6ced435546f20528afb3c00320..e2880ec6fec8d6e0f20e5c60afe4c284de9e6198 100644 (file)
@@ -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)));
index 57b5e50e40d9192b88b0020c5b84f3b9134b6910..78ac73a7418ccf80db053f84b57bd22bd0f6bcd5 100644 (file)
@@ -52,18 +52,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 {
 
index 0227eab0edeb43fb5a44bfb48589140c3caa34cf..f5ba637e58e20332d465c7562d895a14d56f8014 100644 (file)
@@ -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)) {
index c9d2ad5c71b62e889aa031c89357e56c7b2bb626..a46e49ccde83e4622a2558f7ff7c94ee1f801a09 100644 (file)
@@ -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) {
index bdb57f59dde61ce020d69d33ee7fc7fb473351b0..4eac89c37a5b13b13c4a0de58b691cf5810b9550 100644 (file)
@@ -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;
 }
 
index 51b7e7d13c75e7e6afb93fd559a40f161cf6185f..df19ded398d25b1e67bd068873b0be56e7204830 100644 (file)
@@ -199,14 +199,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);
   }
@@ -216,7 +213,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);
 }
@@ -924,20 +921,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);
 
index 08222f787741095f514be7d4e9df180341bd815f..104fb199da0802e0297f36927dae0961a8199399 100644 (file)
@@ -234,8 +234,8 @@ 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) {
+computeGUIDPreservedSymbols(const StringSet<> &PreservedSymbols,
+                            const Triple &TheTriple) {
   DenseSet<GlobalValue::GUID> GUIDPreservedSymbols(PreservedSymbols.size());
   for (auto &Entry : PreservedSymbols) {
     StringRef Name = Entry.first();
@@ -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
index f469c24dd422bf7b5ed2fcbd2c4770551d0527ad..b8fc79a03b6d1d9c863a8baea326d4f13dd810e8 100644 (file)
@@ -650,13 +650,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);
diff --git a/test/ThinLTO/X86/Inputs/weak_autohide.ll b/test/ThinLTO/X86/Inputs/weak_autohide.ll
deleted file mode 100644 (file)
index c9c2bfd..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-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
-}
index 14e7bae6cd9040dc0442e0d87cf5af1da5b47091..6f1cbfe596932602df034cf37556c3ed3c8e610e 100644 (file)
@@ -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
 
diff --git a/test/ThinLTO/X86/weak_autohide.ll b/test/ThinLTO/X86/weak_autohide.ll
deleted file mode 100644 (file)
index 063e36d..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-; 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
-}
-