From 4c45b898f09341727af643ad5fc09379331d77e5 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Tue, 26 Jun 2018 00:20:49 +0000 Subject: [PATCH] [ThinLTO] Compute GUID directly from GV when building per-module index Summary: I discovered when writing the summary parsing support that the per-module index builder and writer are computing the GUID from the value name alone (ignoring the linkage type). This was ok since those GUID were not emitted in the bitcode, and there are never multiple conflicting names in a single module. However, I don't see a reason for making the GUID computation different for the per-module case. It also makes things simpler on the parsing side to have the GUID computation consistent. So this patch changes the summary analysis phase and the per-module summary writer to compute the GUID using the facility on the GlobalValue. Reviewers: pcc, dexonsmith Subscribers: llvm-commits, inglorion Differential Revision: https://reviews.llvm.org/D47844 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@335560 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/ModuleSummaryIndex.h | 9 +++++++-- lib/Analysis/ModuleSummaryAnalysis.cpp | 12 ++++++------ lib/Bitcode/Writer/BitcodeWriter.cpp | 4 ++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/include/llvm/IR/ModuleSummaryIndex.h b/include/llvm/IR/ModuleSummaryIndex.h index 064ca653611..8938001220d 100644 --- a/include/llvm/IR/ModuleSummaryIndex.h +++ b/include/llvm/IR/ModuleSummaryIndex.h @@ -914,6 +914,12 @@ public: std::set &cfiFunctionDecls() { return CfiFunctionDecls; } const std::set &cfiFunctionDecls() const { return CfiFunctionDecls; } + /// Add a global value summary for a value. + void addGlobalValueSummary(const GlobalValue &GV, + std::unique_ptr Summary) { + addGlobalValueSummary(getOrInsertValueInfo(&GV), std::move(Summary)); + } + /// Add a global value summary for a value of the given name. void addGlobalValueSummary(StringRef ValueName, std::unique_ptr Summary) { @@ -965,8 +971,7 @@ public: GlobalValueSummary *getGlobalValueSummary(const GlobalValue &GV, bool PerModuleIndex = true) const { assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name"); - return getGlobalValueSummary(GlobalValue::getGUID(GV.getName()), - PerModuleIndex); + return getGlobalValueSummary(GV.getGUID(), PerModuleIndex); } /// Returns the first GlobalValueSummary for \p ValueGUID, asserting that diff --git a/lib/Analysis/ModuleSummaryAnalysis.cpp b/lib/Analysis/ModuleSummaryAnalysis.cpp index ca0a315bf39..6a589b4ff25 100644 --- a/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -361,7 +361,7 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, TypeCheckedLoadConstVCalls.takeVector()); if (NonRenamableLocal) CantBePromoted.insert(F.getGUID()); - Index.addGlobalValueSummary(F.getName(), std::move(FuncSummary)); + Index.addGlobalValueSummary(F, std::move(FuncSummary)); } static void @@ -377,7 +377,7 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V, llvm::make_unique(Flags, RefEdges.takeVector()); if (NonRenamableLocal) CantBePromoted.insert(V.getGUID()); - Index.addGlobalValueSummary(V.getName(), std::move(GVarSummary)); + Index.addGlobalValueSummary(V, std::move(GVarSummary)); } static void @@ -393,7 +393,7 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A, AS->setAliasee(AliaseeSummary); if (NonRenamableLocal) CantBePromoted.insert(A.getGUID()); - Index.addGlobalValueSummary(A.getName(), std::move(AS)); + Index.addGlobalValueSummary(A, std::move(AS)); } // Set LiveRoot flag on entries matching the given value name. @@ -455,7 +455,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( /* NotEligibleToImport = */ true, /* Live = */ true, /* Local */ GV->isDSOLocal()); - CantBePromoted.insert(GlobalValue::getGUID(Name)); + CantBePromoted.insert(GV->getGUID()); // Create the appropriate summary type. if (Function *F = dyn_cast(GV)) { std::unique_ptr Summary = @@ -472,12 +472,12 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( ArrayRef{}, ArrayRef{}, ArrayRef{}); - Index.addGlobalValueSummary(Name, std::move(Summary)); + Index.addGlobalValueSummary(*GV, std::move(Summary)); } else { std::unique_ptr Summary = llvm::make_unique(GVFlags, ArrayRef{}); - Index.addGlobalValueSummary(Name, std::move(Summary)); + Index.addGlobalValueSummary(*GV, std::move(Summary)); } }); } diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index a3bcf6378cb..5713535ed81 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -3483,7 +3483,7 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord( void ModuleBitcodeWriterBase::writeModuleLevelReferences( const GlobalVariable &V, SmallVector &NameVals, unsigned FSModRefsAbbrev) { - auto VI = Index->getValueInfo(GlobalValue::getGUID(V.getName())); + auto VI = Index->getValueInfo(V.getGUID()); if (!VI || VI.getSummaryList().empty()) { // Only declarations should not have a summary (a declaration might however // have a summary if the def was in module level asm). @@ -3592,7 +3592,7 @@ void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() { if (!F.hasName()) report_fatal_error("Unexpected anonymous function when writing summary"); - ValueInfo VI = Index->getValueInfo(GlobalValue::getGUID(F.getName())); + ValueInfo VI = Index->getValueInfo(F.getGUID()); if (!VI || VI.getSummaryList().empty()) { // Only declarations should not have a summary (a declaration might // however have a summary if the def was in module level asm).