[llvm] r270584 - [ThinLTO] Change ODR resolution and internalization to be index-based

Steven Wu via llvm-commits llvm-commits at lists.llvm.org
Tue May 24 14:29:34 PDT 2016


Hi Teresa

This commit seems cause bot failure: http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_check/7989/

You should be able to see failure if you run ASAN with the test (I am running on my own branch so the line number might not match).
==65822==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000012 (pc 0x00010f73d69d bp 0x7fff52667270 sp 0x7fff52667250 T0)
==65822==The signal is caused by a READ memory access.
==65822==Hint: address points to the zero page.
    #0 0x10f73d69c in std::__1::__function::__func<(anonymous namespace)::resolveWeakForLinkerInIndex(llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long long, std::__1::hash<unsigned long long>, std::__1::equal_to<unsigned long long>, std::__1::allocator<unsigned long long> >, llvm::MallocAllocator> const&, llvm::DenseSet<unsigned long long, llvm::DenseMapInfo<unsigned long long> > const&, llvm::StringMap<std::__1::map<unsigned long long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long long>, std::__1::allocator<std::__1::pair<unsigned long long const, llvm::GlobalValue::LinkageTypes> > >, llvm::MallocAllocator>&)::$_10, std::__1::allocator<(anonymous namespace)::resolveWeakForLinkerInIndex(llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long long, std::__1::hash<unsigned long long>, std::__1::equal_to<unsigned long long>, std::__1::allocator<unsigned long long> >, llvm::MallocAllocator> const&, llvm::DenseSet<unsigned long long, llvm::DenseMapInfo<unsigned long long> > const&, llvm::StringMap<std::__1::map<unsigned long long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long long>, std::__1::allocator<std::__1::pair<unsigned long long const, llvm::GlobalValue::LinkageTypes> > >, llvm::MallocAllocator>&)::$_10>, bool (llvm::StringRef, unsigned long long)>::operator()(llvm::StringRef&&, unsigned long long&&) __hash_table:836
    #1 0x10f732c7e in (anonymous namespace)::resolveWeakForLinkerInIndex(llvm::ModuleSummaryIndex&, llvm::StringMap<std::__1::unordered_set<unsigned long long, std::__1::hash<unsigned long long>, std::__1::equal_to<unsigned long long>, std::__1::allocator<unsigned long long> >, llvm::MallocAllocator> const&, llvm::DenseSet<unsigned long long, llvm::DenseMapInfo<unsigned long long> > const&, llvm::StringMap<std::__1::map<unsigned long long, llvm::GlobalValue::LinkageTypes, std::__1::less<unsigned long long>, std::__1::allocator<std::__1::pair<unsigned long long const, llvm::GlobalValue::LinkageTypes> > >, llvm::MallocAllocator>&) functional:1896
    #2 0x10f730cea in llvm::ThinLTOCodeGenerator::promote(llvm::Module&, llvm::ModuleSummaryIndex&) ThinLTOCodeGenerator.cpp:720
    #3 0x10d5b7237 in thinlto::ThinLTOProcessing::promote() llvm-lto.cpp:525
    #4 0x10d59eb46 in main llvm-lto.cpp:721
    #5 0x7fff8b9445ac in start (libdyld.dylib+0x35ac)
    #6 0x6  (<unknown module>)

Thanks

Steven


> On May 24, 2016, at 10:24 AM, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: tejohnson
> Date: Tue May 24 12:24:25 2016
> New Revision: 270584
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=270584&view=rev
> Log:
> [ThinLTO] Change ODR resolution and internalization to be index-based
> 
> Summary:
> This patch changes the ODR resolution and internalization to be based on
> updates to the Index, which are consumed by the backend portion of the
> transformations.
> 
> It will be followed by an NFC change to move these out of libLTO's
> ThinLTOCodeGenerator so that it can be used by other linkers
> (gold and lld) and by ThinLTO distributed backends.
> 
> The global summary-based portions use callbacks so that the client can
> determine the prevailing copy and other information in a client-specific
> way. Eventually, with the API being developed in D20268, these may be
> modified to use information such as symbol resolutions, supplied by the
> clients to the API.
> 
> Reviewers: joker-eph
> 
> Subscribers: joker.eph, pcc, llvm-commits
> 
> Differential Revision: http://reviews.llvm.org/D20290
> 
> Modified:
>    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
>    llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h
>    llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> 
> Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=270584&r1=270583&r2=270584&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
> +++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Tue May 24 12:24:25 2016
> @@ -168,6 +168,12 @@ public:
>     return static_cast<GlobalValue::LinkageTypes>(Flags.Linkage);
>   }
> 
> +  /// Sets the linkage to the value determined by global summary-based
> +  /// optimization. Will be applied in the ThinLTO backends.
> +  void setLinkage(GlobalValue::LinkageTypes Linkage) {
> +    Flags.Linkage = Linkage;
> +  }
> +
>   /// Return true if this summary is for a GlobalValue that needs promotion
>   /// to be referenced from another module.
>   bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); }
> @@ -446,6 +452,13 @@ public:
>     return NewName.str();
>   }
> 
> +  /// Helper to obtain the unpromoted name for a global value (or the original
> +  /// name if not promoted).
> +  static StringRef getOriginalNameBeforePromote(StringRef Name) {
> +    std::pair<StringRef, StringRef> Pair = Name.split(".llvm.");
> +    return Pair.first;
> +  }
> +
>   /// Add a new module path with the given \p Hash, mapped to the given \p
>   /// ModID, and return an iterator to the entry in the index.
>   ModulePathStringTableTy::iterator
> 
> Modified: llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h?rev=270584&r1=270583&r2=270584&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h (original)
> +++ llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h Tue May 24 12:24:25 2016
> @@ -198,7 +198,9 @@ public:
>   std::unique_ptr<ModuleSummaryIndex> linkCombinedIndex();
> 
>   /**
> -   * Perform promotion and renaming of exported internal functions.
> +   * Perform promotion and renaming of exported internal functions,
> +   * and additionally resolve weak and linkonce symbols.
> +   * Index is updated to reflect linkage changes from weak resolution.
>    */
>   void promote(Module &Module, ModuleSummaryIndex &Index);
> 
> @@ -222,7 +224,7 @@ public:
>       std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
> 
>   /**
> -   * Perform internalization.
> +   * Perform internalization. Index is updated to reflect linkage changes.
>    */
>   void internalize(Module &Module, ModuleSummaryIndex &Index);
> 
> 
> Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=270584&r1=270583&r2=270584&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
> +++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Tue May 24 12:24:25 2016
> @@ -37,7 +37,6 @@
> #include "llvm/MC/SubtargetFeature.h"
> #include "llvm/Object/IRObjectFile.h"
> #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
> -#include "llvm/Support/Debug.h"
> #include "llvm/Support/CachePruning.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/Path.h"
> @@ -89,9 +88,17 @@ static void saveTempBitcode(const Module
>   WriteBitcodeToFile(&TheModule, OS, /* ShouldPreserveUseListOrder */ true);
> }
> 
> -bool IsFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList,
> -                                const ModuleSummaryIndex &Index,
> -                                StringRef ModulePath) {
> +static const GlobalValueSummary *
> +getFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList) {
> +  // If there is any strong definition anywhere, get it.
> +  auto StrongDefForLinker = llvm::find_if(
> +      GVSummaryList, [](const std::unique_ptr<GlobalValueSummary> &Summary) {
> +        auto Linkage = Summary->linkage();
> +        return !GlobalValue::isAvailableExternallyLinkage(Linkage) &&
> +               !GlobalValue::isWeakForLinker(Linkage);
> +      });
> +  if (StrongDefForLinker != GVSummaryList.end())
> +    return StrongDefForLinker->get();
>   // Get the first *linker visible* definition for this global in the summary
>   // list.
>   auto FirstDefForLinker = llvm::find_if(
> @@ -99,78 +106,89 @@ bool IsFirstDefinitionForLinker(const Gl
>         auto Linkage = Summary->linkage();
>         return !GlobalValue::isAvailableExternallyLinkage(Linkage);
>       });
> -  // If \p GV is not the first definition, give up...
> -  if ((*FirstDefForLinker)->modulePath() != ModulePath)
> -    return false;
> -  // If there is any strong definition anywhere, do not bother emitting this.
> -  if (llvm::any_of(
> -          GVSummaryList,
> -          [](const std::unique_ptr<GlobalValueSummary> &Summary) {
> -            auto Linkage = Summary->linkage();
> -            return !GlobalValue::isAvailableExternallyLinkage(Linkage) &&
> -                   !GlobalValue::isWeakForLinker(Linkage);
> -          }))
> -    return false;
> -  return true;
> -}
> -
> -static GlobalValue::LinkageTypes
> -ResolveODR(const ModuleSummaryIndex &Index,
> -           const FunctionImporter::ExportSetTy &ExportList,
> -           const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> -           StringRef ModuleIdentifier, GlobalValue::GUID GUID,
> -           const GlobalValueSummary &GV) {
> +  // Extern templates can be emitted as available_externally.
> +  if (FirstDefForLinker == GVSummaryList.end())
> +    return nullptr;
> +  return FirstDefForLinker->get();
> +}
> +
> +// Populate map of GUID to the prevailing copy for any multiply defined
> +// symbols. Currently assume first copy is prevailing, or any strong
> +// definition. Can be refined with Linker information in the future.
> +static void computePrevailingCopies(
> +    const ModuleSummaryIndex &Index,
> +    DenseMap<GlobalValue::GUID, const GlobalValueSummary *> &PrevailingCopy) {
>   auto HasMultipleCopies = [&](const GlobalValueSummaryList &GVSummaryList) {
>     return GVSummaryList.size() > 1;
>   };
> 
> -  auto OriginalLinkage = GV.linkage();
> -  switch (OriginalLinkage) {
> -  case GlobalValue::ExternalLinkage:
> -  case GlobalValue::AvailableExternallyLinkage:
> -  case GlobalValue::AppendingLinkage:
> -  case GlobalValue::InternalLinkage:
> -  case GlobalValue::PrivateLinkage:
> -  case GlobalValue::ExternalWeakLinkage:
> -  case GlobalValue::CommonLinkage:
> -  case GlobalValue::LinkOnceAnyLinkage:
> -  case GlobalValue::WeakAnyLinkage:
> -    break;
> -  case GlobalValue::LinkOnceODRLinkage:
> -  case GlobalValue::WeakODRLinkage: {
> -    auto &GVSummaryList = Index.findGlobalValueSummaryList(GUID)->second;
> -    // We need to emit only one of these, the first module will keep
> -    // it, but turned into a weak while the others will drop it.
> -    if (!HasMultipleCopies(GVSummaryList)) {
> -      // Exported LinkonceODR needs to be promoted to not be discarded
> -      if (GlobalValue::isDiscardableIfUnused(OriginalLinkage) &&
> -          (ExportList.count(GUID) || GUIDPreservedSymbols.count(GUID)))
> -        return GlobalValue::WeakODRLinkage;
> -      break;
> -    }
> -    if (IsFirstDefinitionForLinker(GVSummaryList, Index, ModuleIdentifier))
> -      return GlobalValue::WeakODRLinkage;
> -    else if (isa<AliasSummary>(&GV))
> -      // Alias can't be turned into available_externally.
> -      return OriginalLinkage;
> -    return GlobalValue::AvailableExternallyLinkage;
> -  }
> -  }
> -  return OriginalLinkage;
> -}
> -
> -/// Resolve LinkOnceODR and WeakODR.
> -///
> -/// We'd like to drop these function if they are no longer referenced in the
> -/// current module. However there is a chance that another module is still
> -/// referencing them because of the import. We make sure we always emit at least
> -/// one copy.
> -static void ResolveODR(
> -    const ModuleSummaryIndex &Index,
> -    const FunctionImporter::ExportSetTy &ExportList,
> -    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> -    const GVSummaryMapTy &DefinedGlobals, StringRef ModuleIdentifier,
> -    std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
> +  for (auto &I : Index) {
> +    if (HasMultipleCopies(I.second))
> +      PrevailingCopy[I.first] = getFirstDefinitionForLinker(I.second);
> +  }
> +}
> +
> +static void thinLTOResolveWeakForLinkerGUID(
> +    GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
> +    DenseSet<GlobalValueSummary *> &GlobalInvolvedWithAlias,
> +    std::function<bool(GlobalValue::GUID, const GlobalValueSummary *)>
> +        isPrevailing,
> +    std::function<bool(StringRef, GlobalValue::GUID)> isExported,
> +    std::function<void(StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes)>
> +        recordNewLinkage) {
> +  auto HasMultipleCopies = GVSummaryList.size() > 1;
> +
> +  for (auto &S : GVSummaryList) {
> +    if (GlobalInvolvedWithAlias.count(S.get()))
> +      continue;
> +    GlobalValue::LinkageTypes OriginalLinkage = S->linkage();
> +    if (!GlobalValue::isWeakForLinker(OriginalLinkage))
> +      continue;
> +    // We need to emit only one of these, the first module will keep it,
> +    // but turned into a weak, while the others will drop it when possible.
> +    if (!HasMultipleCopies) {
> +      // Exported Linkonce needs to be promoted to not be discarded.
> +      // FIXME: This should handle LinkOnceAny as well, but that should be a
> +      // follow-on to the NFC restructuring:
> +      // if (GlobalValue::isLinkOnceLinkage(OriginalLinkage) &&
> +      //     isExported(S->modulePath(), GUID))
> +      //   S->setLinkage(GlobalValue::getWeakLinkage(
> +      //       GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
> +      if (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage) &&
> +          isExported(S->modulePath(), GUID))
> +        S->setLinkage(GlobalValue::WeakODRLinkage);
> +    } else if (isPrevailing(GUID, S.get())) {
> +      // FIXME: This should handle LinkOnceAny as well, but that should be a
> +      // follow-on to the NFC restructuring:
> +      // if (GlobalValue::isLinkOnceLinkage(OriginalLinkage))
> +      //   S->setLinkage(GlobalValue::getWeakLinkage(
> +      //       GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
> +      if (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage))
> +        S->setLinkage(GlobalValue::WeakODRLinkage);
> +    }
> +    // Alias can't be turned into available_externally.
> +    else if (!isa<AliasSummary>(S.get()) &&
> +             (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage) ||
> +              GlobalValue::isWeakODRLinkage(OriginalLinkage)))
> +      S->setLinkage(GlobalValue::AvailableExternallyLinkage);
> +    if (S->linkage() != OriginalLinkage)
> +      recordNewLinkage(S->modulePath(), GUID, S->linkage());
> +  }
> +}
> +
> +// Resolve Weak and LinkOnce values in the \p Index.
> +//
> +// We'd like to drop these functions if they are no longer referenced in the
> +// current module. However there is a chance that another module is still
> +// referencing them because of the import. We make sure we always emit at least
> +// one copy.
> +void thinLTOResolveWeakForLinkerInIndex(
> +    ModuleSummaryIndex &Index,
> +    std::function<bool(GlobalValue::GUID, const GlobalValueSummary *)>
> +        isPrevailing,
> +    std::function<bool(StringRef, GlobalValue::GUID)> isExported,
> +    std::function<void(StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes)>
> +        recordNewLinkage) {
>   if (Index.modulePaths().size() == 1)
>     // Nothing to do if we don't have multiple modules
>     return;
> @@ -179,51 +197,41 @@ static void ResolveODR(
>   // Ideally we should turn the alias into a global and duplicate the definition
>   // when needed.
>   DenseSet<GlobalValueSummary *> GlobalInvolvedWithAlias;
> -  for (auto &GA : DefinedGlobals) {
> -    if (auto AS = dyn_cast<AliasSummary>(GA.second))
> -      GlobalInvolvedWithAlias.insert(&AS->getAliasee());
> -  }
> -
> -  for (auto &GV : DefinedGlobals) {
> -    if (GlobalInvolvedWithAlias.count(GV.second))
> -      continue;
> -    auto NewLinkage =
> -        ResolveODR(Index, ExportList, GUIDPreservedSymbols, ModuleIdentifier, GV.first, *GV.second);
> -    if (NewLinkage != GV.second->linkage()) {
> -      ResolvedODR[GV.first] = NewLinkage;
> -    }
> -  }
> +  for (auto &I : Index)
> +    for (auto &S : I.second)
> +      if (auto AS = dyn_cast<AliasSummary>(S.get()))
> +        GlobalInvolvedWithAlias.insert(&AS->getAliasee());
> +
> +  for (auto &I : Index)
> +    thinLTOResolveWeakForLinkerGUID(I.second, I.first, GlobalInvolvedWithAlias,
> +                                    isPrevailing, isExported, recordNewLinkage);
> }
> 
> -/// Fixup linkage, see ResolveODR() above.
> -void fixupODR(
> -    Module &TheModule,
> -    const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
> -  // Process functions and global now
> -  for (auto &GV : TheModule) {
> -    auto NewLinkage = ResolvedODR.find(GV.getGUID());
> -    if (NewLinkage == ResolvedODR.end())
> -      continue;
> -    DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
> -                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
> -    GV.setLinkage(NewLinkage->second);
> -  }
> -  for (auto &GV : TheModule.globals()) {
> -    auto NewLinkage = ResolvedODR.find(GV.getGUID());
> -    if (NewLinkage == ResolvedODR.end())
> -      continue;
> -    DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
> -                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
> -    GV.setLinkage(NewLinkage->second);
> -  }
> -  for (auto &GV : TheModule.aliases()) {
> -    auto NewLinkage = ResolvedODR.find(GV.getGUID());
> -    if (NewLinkage == ResolvedODR.end())
> -      continue;
> +/// Fixup WeakForLinker linkages in \p TheModule based on summary analysis.
> +void thinLTOResolveWeakForLinkerModule(Module &TheModule,
> +                                       const GVSummaryMapTy &DefinedGlobals) {
> +  auto updateLinkage = [&](GlobalValue &GV) {
> +    if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
> +      return;
> +    // See if the global summary analysis computed a new resolved linkage.
> +    const auto &GS = DefinedGlobals.find(GV.getGUID());
> +    if (GS == DefinedGlobals.end())
> +      return;
> +    auto NewLinkage = GS->second->linkage();
> +    if (NewLinkage == GV.getLinkage())
> +      return;
>     DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
> -                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
> -    GV.setLinkage(NewLinkage->second);
> -  }
> +                 << GV.getLinkage() << " to " << NewLinkage << "\n");
> +    GV.setLinkage(NewLinkage);
> +  };
> +
> +  // Process functions and global now
> +  for (auto &GV : TheModule)
> +    updateLinkage(GV);
> +  for (auto &GV : TheModule.globals())
> +    updateLinkage(GV);
> +  for (auto &GV : TheModule.aliases())
> +    updateLinkage(GV);
> }
> 
> static StringMap<MemoryBufferRef>
> @@ -276,41 +284,30 @@ static void optimizeModule(Module &TheMo
>   PM.run(TheModule);
> }
> 
> -// Create a DenseSet of GlobalValue to be used with the Internalizer.
> -static DenseSet<const GlobalValue *> computePreservedSymbolsForModule(
> -    Module &TheModule, const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> -    const FunctionImporter::ExportSetTy &ExportList) {
> -  DenseSet<const GlobalValue *> PreservedGV;
> -  if (GUIDPreservedSymbols.empty())
> -    // Early exit: internalize is disabled when there is nothing to preserve.
> -    return PreservedGV;
> -
> -  auto AddPreserveGV = [&](const GlobalValue &GV) {
> -    auto GUID = GV.getGUID();
> -    if (GUIDPreservedSymbols.count(GUID) || ExportList.count(GUID))
> -      PreservedGV.insert(&GV);
> -  };
> -
> -  for (auto &GV : TheModule)
> -    AddPreserveGV(GV);
> -  for (auto &GV : TheModule.globals())
> -    AddPreserveGV(GV);
> -  for (auto &GV : TheModule.aliases())
> -    AddPreserveGV(GV);
> -
> -  return PreservedGV;
> -}
> -
> -// Run internalization on \p TheModule
> -static void
> -doInternalizeModule(Module &TheModule, const TargetMachine &TM,
> -                    const DenseSet<const GlobalValue *> &PreservedGV) {
> -  if (PreservedGV.empty()) {
> -    // Be friendly and don't nuke totally the module when the client didn't
> -    // supply anything to preserve.
> -    return;
> +static void thinLTOInternalizeAndPromoteGUID(
> +    GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
> +    std::function<bool(StringRef, GlobalValue::GUID)> isExported) {
> +  for (auto &S : GVSummaryList) {
> +    if (isExported(S->modulePath(), GUID)) {
> +      if (GlobalValue::isLocalLinkage(S->linkage()))
> +        S->setLinkage(GlobalValue::ExternalLinkage);
> +    } else if (!GlobalValue::isLocalLinkage(S->linkage()))
> +      S->setLinkage(GlobalValue::InternalLinkage);
>   }
> +}
> 
> +// Update the linkages in the given \p Index to mark exported values
> +// as external and non-exported values as internal.
> +void thinLTOInternalizeAndPromoteInIndex(
> +    ModuleSummaryIndex &Index,
> +    std::function<bool(StringRef, GlobalValue::GUID)> isExported) {
> +  for (auto &I : Index)
> +    thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported);
> +}
> +
> +// Run internalization on \p TheModule based on symmary analysis.
> +void thinLTOInternalizeModule(Module &TheModule,
> +                              const GVSummaryMapTy &DefinedGlobals) {
>   // Parse inline ASM and collect the list of symbols that are not defined in
>   // the current module.
>   StringSet<> AsmUndefinedRefs;
> @@ -321,15 +318,37 @@ doInternalizeModule(Module &TheModule, c
>           AsmUndefinedRefs.insert(Name);
>       });
> 
> -  // Update the llvm.compiler_used globals to force preserving libcalls and
> -  // symbols referenced from asm
> -  UpdateCompilerUsed(TheModule, TM, AsmUndefinedRefs);
> -
>   // Declare a callback for the internalize pass that will ask for every
>   // candidate GlobalValue if it can be internalized or not.
> -  auto MustPreserveGV =
> -      [&](const GlobalValue &GV) -> bool { return PreservedGV.count(&GV); };
> +  auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {
> +    // Can't be internalized if referenced in inline asm.
> +    if (AsmUndefinedRefs.count(GV.getName()))
> +      return true;
> +
> +    // Lookup the linkage recorded in the summaries during global analysis.
> +    const auto &GS = DefinedGlobals.find(GV.getGUID());
> +    GlobalValue::LinkageTypes Linkage;
> +    if (GS == DefinedGlobals.end()) {
> +      // Must have been promoted (possibly conservatively). Find original
> +      // name so that we can access the correct summary and see if it can
> +      // be internalized again.
> +      // FIXME: Eventually we should control promotion instead of promoting
> +      // and internalizing again.
> +      StringRef OrigName =
> +          ModuleSummaryIndex::getOriginalNameBeforePromote(GV.getName());
> +      std::string OrigId = GlobalValue::getGlobalIdentifier(
> +          OrigName, GlobalValue::InternalLinkage,
> +          TheModule.getSourceFileName());
> +      const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigId));
> +      assert(GS != DefinedGlobals.end());
> +      Linkage = GS->second->linkage();
> +    } else
> +      Linkage = GS->second->linkage();
> +    return !GlobalValue::isLocalLinkage(Linkage);
> +  };
> 
> +  // FIXME: See if we can just internalize directly here via linkage changes
> +  // based on the index, rather than invoking internalizeModule.
>   llvm::internalizeModule(TheModule, MustPreserveGV);
> }
> 
> @@ -483,22 +502,16 @@ public:
>   }
> };
> 
> -static std::unique_ptr<MemoryBuffer> ProcessThinLTOModule(
> -    Module &TheModule, const ModuleSummaryIndex &Index,
> -    StringMap<MemoryBufferRef> &ModuleMap, TargetMachine &TM,
> -    const FunctionImporter::ImportMapTy &ImportList,
> -    const FunctionImporter::ExportSetTy &ExportList,
> -    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> -    std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
> -    ThinLTOCodeGenerator::CachingOptions CacheOptions, bool DisableCodeGen,
> -    StringRef SaveTempsDir, unsigned count) {
> -
> -  // Prepare for internalization by computing the set of symbols to preserve.
> -  // We need to compute the list of symbols to preserve during internalization
> -  // before doing any promotion because after renaming we won't (easily) match
> -  // to the original name.
> -  auto PreservedGV = computePreservedSymbolsForModule(
> -      TheModule, GUIDPreservedSymbols, ExportList);
> +static std::unique_ptr<MemoryBuffer>
> +ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
> +                     StringMap<MemoryBufferRef> &ModuleMap, TargetMachine &TM,
> +                     const FunctionImporter::ImportMapTy &ImportList,
> +                     const FunctionImporter::ExportSetTy &ExportList,
> +                     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> +                     const GVSummaryMapTy &DefinedGlobals,
> +                     ThinLTOCodeGenerator::CachingOptions CacheOptions,
> +                     bool DisableCodeGen, StringRef SaveTempsDir,
> +                     unsigned count) {
> 
>   // "Benchmark"-like optimization: single-source case
>   bool SingleModule = (ModuleMap.size() == 1);
> @@ -506,17 +519,19 @@ static std::unique_ptr<MemoryBuffer> Pro
>   if (!SingleModule) {
>     promoteModule(TheModule, Index);
> 
> -    // Resolve the LinkOnce/Weak ODR, trying to turn them into
> -    // "available_externally" when possible.
> -    // This is a compile-time optimization.
> -    fixupODR(TheModule, ResolvedODR);
> +    // Apply summary-based LinkOnce/Weak resolution decisions.
> +    thinLTOResolveWeakForLinkerModule(TheModule, DefinedGlobals);
> 
>     // Save temps: after promotion.
>     saveTempBitcode(TheModule, SaveTempsDir, count, ".1.promoted.bc");
>   }
> 
> -  // Internalization
> -  doInternalizeModule(TheModule, TM, PreservedGV);
> +  // Be friendly and don't nuke totally the module when the client didn't
> +  // supply anything to preserve.
> +  if (!ExportList.empty() || !GUIDPreservedSymbols.empty()) {
> +    // Apply summary-based internalization decisions.
> +    thinLTOInternalizeModule(TheModule, DefinedGlobals);
> +  }
> 
>   // Save internalized bitcode
>   saveTempBitcode(TheModule, SaveTempsDir, count, ".2.internalized.bc");
> @@ -546,6 +561,45 @@ static std::unique_ptr<MemoryBuffer> Pro
>   return codegenModule(TheModule, TM);
> }
> 
> +/// Resolve LinkOnce/Weak symbols. Record resolutions in the \p ResolvedODR map
> +/// for caching, and in the \p Index for application during the ThinLTO
> +/// backends. This is needed for correctness for exported symbols (ensure
> +/// at least one copy kept) and a compile-time optimization (to drop duplicate
> +/// copies when possible).
> +static void resolveWeakForLinkerInIndex(
> +    ModuleSummaryIndex &Index,
> +    const StringMap<FunctionImporter::ExportSetTy> &ExportLists,
> +    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
> +    StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>>
> +        &ResolvedODR) {
> +
> +  DenseMap<GlobalValue::GUID, const GlobalValueSummary *> PrevailingCopy;
> +  computePrevailingCopies(Index, PrevailingCopy);
> +
> +  auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) {
> +    const auto &Prevailing = PrevailingCopy.find(GUID);
> +    // Not in map means that there was only one copy, which must be prevailing.
> +    if (Prevailing == PrevailingCopy.end())
> +      return true;
> +    return Prevailing->second == S;
> +  };
> +
> +  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
> +    const auto &ExportList = ExportLists.find(ModuleIdentifier);
> +    assert(ExportList != ExportLists.end() && "Missing export list for module");
> +    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
> +  };
> +
> +  auto recordNewLinkage = [&](StringRef ModuleIdentifier,
> +                              GlobalValue::GUID GUID,
> +                              GlobalValue::LinkageTypes NewLinkage) {
> +    ResolvedODR[ModuleIdentifier][GUID] = NewLinkage;
> +  };
> +
> +  thinLTOResolveWeakForLinkerInIndex(Index, isPrevailing, isExported,
> +                                     recordNewLinkage);
> +}
> +
> // Initialize the TargetMachine builder for a given Triple
> static void initTMBuilder(TargetMachineBuilder &TMBuilder,
>                           const Triple &TheTriple) {
> @@ -641,6 +695,7 @@ std::unique_ptr<ModuleSummaryIndex> Thin
> 
> /**
>  * Perform promotion and renaming of exported internal functions.
> + * Index is updated to reflect linkage changes from weak resolution.
>  */
> void ThinLTOCodeGenerator::promote(Module &TheModule,
>                                    ModuleSummaryIndex &Index) {
> @@ -655,21 +710,18 @@ void ThinLTOCodeGenerator::promote(Modul
>   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
>   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
>                            ExportLists);
> -  auto &ExportList = ExportLists[ModuleIdentifier];
> 
>   // Convert the preserved symbols set from string to GUID
>   auto GUIDPreservedSymbols =
>   computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
> 
> -  // Resolve the LinkOnceODR, trying to turn them into "available_externally"
> -  // where possible.
> -  // This is a compile-time optimization.
> -  // We use a std::map here to be able to have a defined ordering when
> -  // producing a hash for the cache entry.
> -  std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
> -  ResolveODR(Index, ExportList, GUIDPreservedSymbols, ModuleToDefinedGVSummaries[ModuleIdentifier],
> -             ModuleIdentifier, ResolvedODR);
> -  fixupODR(TheModule, ResolvedODR);
> +  // Resolve LinkOnce/Weak symbols.
> +  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
> +  resolveWeakForLinkerInIndex(Index, ExportLists, GUIDPreservedSymbols,
> +                              ResolvedODR);
> +
> +  thinLTOResolveWeakForLinkerModule(
> +      TheModule, ModuleToDefinedGVSummaries[ModuleIdentifier]);
> 
>   promoteModule(TheModule, Index);
> }
> @@ -744,7 +796,7 @@ void ThinLTOCodeGenerator::emitImports(S
> }
> 
> /**
> - * Perform internalization.
> + * Perform internalization. Index is updated to reflect linkage changes.
>  */
> void ThinLTOCodeGenerator::internalize(Module &TheModule,
>                                        ModuleSummaryIndex &Index) {
> @@ -767,10 +819,20 @@ void ThinLTOCodeGenerator::internalize(M
>                            ExportLists);
>   auto &ExportList = ExportLists[ModuleIdentifier];
> 
> +  // Be friendly and don't nuke totally the module when the client didn't
> +  // supply anything to preserve.
> +  if (ExportList.empty() && GUIDPreservedSymbols.empty())
> +    return;
> +
>   // Internalization
> -  auto PreservedGV = computePreservedSymbolsForModule(
> -      TheModule, GUIDPreservedSymbols, ExportList);
> -  doInternalizeModule(TheModule, *TMBuilder.create(), PreservedGV);
> +  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
> +    const auto &ExportList = ExportLists.find(ModuleIdentifier);
> +    assert(ExportList != ExportLists.end() && "Missing export list for module");
> +    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
> +  };
> +  thinLTOInternalizeAndPromoteInIndex(Index, isExported);
> +  thinLTOInternalizeModule(TheModule,
> +                           ModuleToDefinedGVSummaries[ModuleIdentifier]);
> }
> 
> /**
> @@ -853,6 +915,28 @@ void ThinLTOCodeGenerator::run() {
>   auto GUIDPreservedSymbols =
>       computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
> 
> +  // We use a std::map here to be able to have a defined ordering when
> +  // producing a hash for the cache entry.
> +  // FIXME: we should be able to compute the caching hash for the entry based
> +  // on the index, and nuke this map.
> +  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
> +
> +  // Resolve LinkOnce/Weak symbols, this has to be computed early because it
> +  // impacts the caching.
> +  resolveWeakForLinkerInIndex(*Index, ExportLists, GUIDPreservedSymbols,
> +                              ResolvedODR);
> +
> +  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
> +    const auto &ExportList = ExportLists.find(ModuleIdentifier);
> +    assert(ExportList != ExportLists.end() && "Missing export list for module");
> +    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
> +  };
> +
> +  // Use global summary-based analysis to identify symbols that can be
> +  // internalized (because they aren't exported or preserved as per callback).
> +  // Changes are made in the index, consumed in the ThinLTO backends.
> +  thinLTOInternalizeAndPromoteInIndex(*Index, isExported);
> +
>   // Make sure that every module has an entry in the ExportLists to enable
>   // threaded access to this map below
>   for (auto &DefinedGVSummaries : ModuleToDefinedGVSummaries)
> @@ -882,18 +966,11 @@ void ThinLTOCodeGenerator::run() {
> 
>         auto &DefinedFunctions = ModuleToDefinedGVSummaries[ModuleIdentifier];
> 
> -        // Resolve ODR, this has to be done early because it impacts the caching
> -        // We use a std::map here to be able to have a defined ordering when
> -        // producing a hash for the cache entry.
> -        std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
> -        ResolveODR(*Index, ExportList, GUIDPreservedSymbols, DefinedFunctions, ModuleIdentifier,
> -                   ResolvedODR);
> -
>         // The module may be cached, this helps handling it.
>         ModuleCacheEntry CacheEntry(CacheOptions.Path, *Index, ModuleIdentifier,
>                                     ImportLists[ModuleIdentifier], ExportList,
> -                                    ResolvedODR, DefinedFunctions,
> -                                    GUIDPreservedSymbols);
> +                                    ResolvedODR[ModuleIdentifier],
> +                                    DefinedFunctions, GUIDPreservedSymbols);
> 
>         {
>           auto ErrOrBuffer = CacheEntry.tryLoadingBuffer();
> @@ -922,7 +999,8 @@ void ThinLTOCodeGenerator::run() {
>         // Run the main process now, and generates a binary
>         auto OutputBuffer = ProcessThinLTOModule(
>             *TheModule, *Index, ModuleMap, *TMBuilder.create(), ImportList,
> -            ExportList, GUIDPreservedSymbols, ResolvedODR, CacheOptions,
> +            ExportList, GUIDPreservedSymbols,
> +            ModuleToDefinedGVSummaries[ModuleIdentifier], CacheOptions,
>             DisableCodeGen, SaveTempsDir, count);
> 
>         OutputBuffer = CacheEntry.write(std::move(OutputBuffer));
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list