From 87de206ac474c0b2e953cf41cca5863f151044c4 Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Sat, 23 Apr 2016 23:38:17 +0000 Subject: [PATCH] Store and emit original name in combined index Summary: As discussed in D18298, some local globals can't be renamed/promoted (because they have a section, or because they are referenced from inline assembly). To be able to detect naming collision, we need to keep around the "GUID" using their original name without taking the linkage into account. Reviewers: tejohnson Subscribers: joker.eph, llvm-commits Differential Revision: http://reviews.llvm.org/D19454 From: Mehdi Amini llvm-svn: 267304 --- include/llvm/Bitcode/LLVMBitCodes.h | 2 + include/llvm/IR/ModuleSummaryIndex.h | 12 +++ lib/Bitcode/Reader/BitcodeReader.cpp | 88 +++++++++++++------ lib/Bitcode/Writer/BitcodeWriter.cpp | 14 +++ .../thinlto-function-summary-originalnames.ll | 24 +++++ tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp | 1 + 6 files changed, 116 insertions(+), 25 deletions(-) create mode 100644 test/Bitcode/thinlto-function-summary-originalnames.ll diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h index 14ba1586e48..71f3cc80dd7 100644 --- a/include/llvm/Bitcode/LLVMBitCodes.h +++ b/include/llvm/Bitcode/LLVMBitCodes.h @@ -217,6 +217,8 @@ enum GlobalValueSummarySymtabCodes { FS_ALIAS = 7, // COMBINED_ALIAS: [modid, linkage, offset] FS_COMBINED_ALIAS = 8, + // COMBINED_ORIGINAL_NAME: [original_name_hash] + FS_COMBINED_ORIGINAL_NAME = 9, }; enum MetadataCodes { diff --git a/include/llvm/IR/ModuleSummaryIndex.h b/include/llvm/IR/ModuleSummaryIndex.h index 41c567a2b0d..e05ef67ded2 100644 --- a/include/llvm/IR/ModuleSummaryIndex.h +++ b/include/llvm/IR/ModuleSummaryIndex.h @@ -96,6 +96,11 @@ private: /// Kind of summary for use in dyn_cast<> et al. SummaryKind Kind; + /// This is the hash of the name of the symbol in the original file. It is + /// identical to the GUID for global symbols, but differs for local since the + /// GUID includes the module level id in the hash. + GlobalValue::GUID OriginalName; + /// \brief Path of module IR containing value's definition, used to locate /// module during importing. /// @@ -128,6 +133,13 @@ protected: public: virtual ~GlobalValueSummary() = default; + /// Returns the hash of the original name, it is identical to the GUID for + /// externally visible symbols, but not for local ones. + GlobalValue::GUID getOriginalName() { return OriginalName; } + + /// Initialize the original name hash in this summary. + void setOriginalName(GlobalValue::GUID Name) { OriginalName = Name; } + /// Which kind of summary subclass this is. SummaryKind getSummaryKind() const { return Kind; } diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 521256005cb..cbacf8127d4 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -487,7 +487,10 @@ class ModuleSummaryIndexBitcodeReader { // call graph edges read from the function summary from referencing // callees by their ValueId to using the GUID instead, which is how // they are recorded in the summary index being built. - DenseMap ValueIdToCallGraphGUIDMap; + // We save a second GUID which is the same as the first one, but ignoring the + // linkage, i.e. for value other than local linkage they are identical. + DenseMap> + ValueIdToCallGraphGUIDMap; /// Map to save the association between summary offset in the VST to the /// GlobalValueInfo object created when parsing it. Used to access the @@ -543,7 +546,8 @@ private: std::error_code initStream(std::unique_ptr Streamer); std::error_code initStreamFromBuffer(); std::error_code initLazyStream(std::unique_ptr Streamer); - GlobalValue::GUID getGUIDFromValueId(unsigned ValueId); + std::pair + getGUIDFromValueId(unsigned ValueId); GlobalValueInfo *getInfoFromSummaryOffset(uint64_t Offset); }; } // end anonymous namespace @@ -5699,7 +5703,7 @@ void ModuleSummaryIndexBitcodeReader::freeState() { Buffer = nullptr; } void ModuleSummaryIndexBitcodeReader::releaseBuffer() { Buffer.release(); } -GlobalValue::GUID +std::pair ModuleSummaryIndexBitcodeReader::getGUIDFromValueId(unsigned ValueId) { auto VGI = ValueIdToCallGraphGUIDMap.find(ValueId); assert(VGI != ValueIdToCallGraphGUIDMap.end()); @@ -5763,13 +5767,19 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseValueSymbolTable( auto VLI = ValueIdToLinkageMap.find(ValueID); assert(VLI != ValueIdToLinkageMap.end() && "No linkage found for VST entry?"); - std::string GlobalId = GlobalValue::getGlobalIdentifier( - ValueName, VLI->second, SourceFileName); + auto Linkage = VLI->second; + std::string GlobalId = + GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName); auto ValueGUID = GlobalValue::getGUID(GlobalId); + auto OriginalNameID = ValueGUID; + if (GlobalValue::isLocalLinkage(Linkage)) + OriginalNameID = GlobalValue::getGUID(ValueName); if (PrintSummaryGUIDs) - dbgs() << "GUID " << ValueGUID << " is " << ValueName << "\n"; + dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is " + << ValueName << "\n"; TheIndex->addGlobalValueInfo(ValueGUID, std::move(GlobalValInfo)); - ValueIdToCallGraphGUIDMap[ValueID] = ValueGUID; + ValueIdToCallGraphGUIDMap[ValueID] = + std::make_pair(ValueGUID, OriginalNameID); ValueName.clear(); break; } @@ -5785,13 +5795,19 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseValueSymbolTable( auto VLI = ValueIdToLinkageMap.find(ValueID); assert(VLI != ValueIdToLinkageMap.end() && "No linkage found for VST entry?"); + auto Linkage = VLI->second; std::string FunctionGlobalId = GlobalValue::getGlobalIdentifier( ValueName, VLI->second, SourceFileName); auto FunctionGUID = GlobalValue::getGUID(FunctionGlobalId); + auto OriginalNameID = FunctionGUID; + if (GlobalValue::isLocalLinkage(Linkage)) + OriginalNameID = GlobalValue::getGUID(ValueName); if (PrintSummaryGUIDs) - dbgs() << "GUID " << FunctionGUID << " is " << ValueName << "\n"; + dbgs() << "GUID " << FunctionGUID << "(" << OriginalNameID << ") is " + << ValueName << "\n"; TheIndex->addGlobalValueInfo(FunctionGUID, std::move(FuncInfo)); - ValueIdToCallGraphGUIDMap[ValueID] = FunctionGUID; + ValueIdToCallGraphGUIDMap[ValueID] = + std::make_pair(FunctionGUID, OriginalNameID); ValueName.clear(); break; @@ -5805,14 +5821,19 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseValueSymbolTable( llvm::make_unique(GlobalValSummaryOffset); SummaryOffsetToInfoMap[GlobalValSummaryOffset] = GlobalValInfo.get(); TheIndex->addGlobalValueInfo(GlobalValGUID, std::move(GlobalValInfo)); - ValueIdToCallGraphGUIDMap[ValueID] = GlobalValGUID; + // The "original name", which is the second value of the pair will be + // overriden later by a FS_COMBINED_ORIGINAL_NAME in the combined index. + ValueIdToCallGraphGUIDMap[ValueID] = + std::make_pair(GlobalValGUID, GlobalValGUID); break; } case bitc::VST_CODE_COMBINED_ENTRY: { // VST_CODE_COMBINED_ENTRY: [valueid, refguid] unsigned ValueID = Record[0]; GlobalValue::GUID RefGUID = Record[1]; - ValueIdToCallGraphGUIDMap[ValueID] = RefGUID; + // The "original name", which is the second value of the pair will be + // overriden later by a FS_COMBINED_ORIGINAL_NAME in the combined index. + ValueIdToCallGraphGUIDMap[ValueID] = std::make_pair(RefGUID, RefGUID); break; } } @@ -5976,7 +5997,9 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { return error("Invalid record"); SmallVector Record; - + // Keep around the last seen summary to be used when we see an optional + // "OriginalName" attachement. + GlobalValueSummary *LastSeenSummary = nullptr; bool Combined = false; while (1) { BitstreamEntry Entry = Stream.advanceSkippingSubblocks(); @@ -6041,7 +6064,7 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { "Record size inconsistent with number of references"); for (unsigned I = 4, E = CallGraphEdgeStartIndex; I != E; ++I) { unsigned RefValueId = Record[I]; - GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId); + GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first; FS->addRefEdge(RefGUID); } bool HasProfile = (BitCode == bitc::FS_PERMODULE_PROFILE); @@ -6050,12 +6073,13 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { unsigned CalleeValueId = Record[I]; unsigned CallsiteCount = Record[++I]; uint64_t ProfileCount = HasProfile ? Record[++I] : 0; - GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId); + GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId).first; FS->addCallGraphEdge(CalleeGUID, CalleeInfo(CallsiteCount, ProfileCount)); } - GlobalValue::GUID GUID = getGUIDFromValueId(ValueID); - auto *Info = TheIndex->getGlobalValueInfo(GUID); + auto GUID = getGUIDFromValueId(ValueID); + FS->setOriginalName(GUID.second); + auto *Info = TheIndex->getGlobalValueInfo(GUID.first); assert(!Info->summary() && "Expected a single summary per VST entry"); Info->setSummary(std::move(FS)); break; @@ -6077,14 +6101,15 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { AS->setModulePath( TheIndex->addModulePath(Buffer->getBufferIdentifier(), 0)->first()); - GlobalValue::GUID AliaseeGUID = getGUIDFromValueId(AliaseeID); + GlobalValue::GUID AliaseeGUID = getGUIDFromValueId(AliaseeID).first; auto *AliaseeInfo = TheIndex->getGlobalValueInfo(AliaseeGUID); if (!AliaseeInfo->summary()) return error("Alias expects aliasee summary to be parsed"); AS->setAliasee(AliaseeInfo->summary()); - GlobalValue::GUID GUID = getGUIDFromValueId(ValueID); - auto *Info = TheIndex->getGlobalValueInfo(GUID); + auto GUID = getGUIDFromValueId(ValueID); + AS->setOriginalName(GUID.second); + auto *Info = TheIndex->getGlobalValueInfo(GUID.first); assert(!Info->summary() && "Expected a single summary per VST entry"); Info->setSummary(std::move(AS)); break; @@ -6099,11 +6124,12 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { TheIndex->addModulePath(Buffer->getBufferIdentifier(), 0)->first()); for (unsigned I = 2, E = Record.size(); I != E; ++I) { unsigned RefValueId = Record[I]; - GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId); + GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first; FS->addRefEdge(RefGUID); } - GlobalValue::GUID GUID = getGUIDFromValueId(ValueID); - auto *Info = TheIndex->getGlobalValueInfo(GUID); + auto GUID = getGUIDFromValueId(ValueID); + FS->setOriginalName(GUID.second); + auto *Info = TheIndex->getGlobalValueInfo(GUID.first); assert(!Info->summary() && "Expected a single summary per VST entry"); Info->setSummary(std::move(FS)); break; @@ -6121,6 +6147,7 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { unsigned NumRefs = Record[3]; std::unique_ptr FS = llvm::make_unique( getDecodedLinkage(RawLinkage), InstCount); + LastSeenSummary = FS.get(); FS->setModulePath(ModuleIdMap[ModuleId]); static int RefListStartIndex = 4; int CallGraphEdgeStartIndex = RefListStartIndex + NumRefs; @@ -6128,7 +6155,7 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { "Record size inconsistent with number of references"); for (unsigned I = 4, E = CallGraphEdgeStartIndex; I != E; ++I) { unsigned RefValueId = Record[I]; - GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId); + GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first; FS->addRefEdge(RefGUID); } bool HasProfile = (BitCode == bitc::FS_COMBINED_PROFILE); @@ -6137,7 +6164,7 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { unsigned CalleeValueId = Record[I]; unsigned CallsiteCount = Record[++I]; uint64_t ProfileCount = HasProfile ? Record[++I] : 0; - GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId); + GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId).first; FS->addCallGraphEdge(CalleeGUID, CalleeInfo(CallsiteCount, ProfileCount)); } @@ -6156,6 +6183,7 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { uint64_t AliaseeSummaryOffset = Record[2]; std::unique_ptr AS = llvm::make_unique(getDecodedLinkage(RawLinkage)); + LastSeenSummary = AS.get(); AS->setModulePath(ModuleIdMap[ModuleId]); auto *AliaseeInfo = getInfoFromSummaryOffset(AliaseeSummaryOffset); @@ -6175,10 +6203,11 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { uint64_t RawLinkage = Record[1]; std::unique_ptr FS = llvm::make_unique(getDecodedLinkage(RawLinkage)); + LastSeenSummary = FS.get(); FS->setModulePath(ModuleIdMap[ModuleId]); for (unsigned I = 2, E = Record.size(); I != E; ++I) { unsigned RefValueId = Record[I]; - GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId); + GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first; FS->addRefEdge(RefGUID); } auto *Info = getInfoFromSummaryOffset(CurRecordBit); @@ -6187,6 +6216,15 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseEntireSummary() { Combined = true; break; } + // FS_COMBINED_ORIGINAL_NAME: [original_name] + case bitc::FS_COMBINED_ORIGINAL_NAME: { + uint64_t OriginalName = Record[0]; + if (!LastSeenSummary) + return error("Name attachment that does not follow a combined record"); + LastSeenSummary->setOriginalName(OriginalName); + // Reset the LastSeenSummary + LastSeenSummary = nullptr; + } } } llvm_unreachable("Exit infinite loop"); diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 3be4fe03d51..302629b9176 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -3211,6 +3211,17 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() { DenseMap SummaryToOffsetMap; SmallVector NameVals; + + // For local linkage, we also emit the original name separately + // immediately after the record. + auto MaybeEmitOriginalName = [&](GlobalValueSummary &S) { + if (!GlobalValue::isLocalLinkage(S.linkage())) + return; + NameVals.push_back(S.getOriginalName()); + Stream.EmitRecord(bitc::FS_COMBINED_ORIGINAL_NAME, NameVals); + NameVals.clear(); + }; + for (const auto &FII : Index) { for (auto &FI : FII.second) { GlobalValueSummary *S = FI->summary(); @@ -3241,6 +3252,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() { Stream.EmitRecord(bitc::FS_COMBINED_GLOBALVAR_INIT_REFS, NameVals, FSModRefsAbbrev); NameVals.clear(); + MaybeEmitOriginalName(*S); continue; } @@ -3288,6 +3300,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() { // Emit the finished record. Stream.EmitRecord(Code, NameVals, FSAbbrev); NameVals.clear(); + MaybeEmitOriginalName(*S); } } @@ -3307,6 +3320,7 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() { // Emit the finished record. Stream.EmitRecord(bitc::FS_COMBINED_ALIAS, NameVals, FSAliasAbbrev); NameVals.clear(); + MaybeEmitOriginalName(*AS); } Stream.ExitBlock(); diff --git a/test/Bitcode/thinlto-function-summary-originalnames.ll b/test/Bitcode/thinlto-function-summary-originalnames.ll new file mode 100644 index 00000000000..431ce367e9b --- /dev/null +++ b/test/Bitcode/thinlto-function-summary-originalnames.ll @@ -0,0 +1,24 @@ +; Test to check the callgraph in summary +; RUN: opt -module-summary %s -o %t.o +; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t.o +; RUN: llvm-bcanalyzer -dump %t.index.bc | FileCheck %s --check-prefix=COMBINED + +; COMBINED: +; COMBINED-NEXT: +; COMBINED-NEXT: +; COMBINED-NEXT: + +; ModuleID = 'thinlto-function-summary-callgraph.ll' +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +@bar = internal global i32 0 +@fooalias = internal alias void (...), bitcast (void ()* @foo to void (...)*) + +define internal void @foo() { + ret void +} diff --git a/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp b/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp index 2ac035bcdf8..0ce83c72f24 100644 --- a/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp +++ b/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp @@ -308,6 +308,7 @@ static const char *GetCodeName(unsigned CodeID, unsigned BlockID, STRINGIFY_CODE(FS, COMBINED_GLOBALVAR_INIT_REFS) STRINGIFY_CODE(FS, ALIAS) STRINGIFY_CODE(FS, COMBINED_ALIAS) + STRINGIFY_CODE(FS, COMBINED_ORIGINAL_NAME) } case bitc::METADATA_ATTACHMENT_ID: switch(CodeID) {