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