diff --git a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h --- a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h +++ b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h @@ -51,7 +51,8 @@ /// StackSafety assumes that missing parameter information means possibility /// of access to the parameter with any offset, so we can correctly link /// code without StackSafety information, e.g. non-ThinLTO. - std::vector getParamAccesses() const; + std::vector + getParamAccesses(ModuleSummaryIndex &Index) const; }; class StackSafetyGlobalInfo { diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -562,12 +562,11 @@ /// offsets from the beginning of the value that are passed. struct Call { uint64_t ParamNo = 0; - GlobalValue::GUID Callee = 0; + ValueInfo Callee; ConstantRange Offsets{/*BitWidth=*/RangeWidth, /*isFullSet=*/true}; Call() = default; - Call(uint64_t ParamNo, GlobalValue::GUID Callee, - const ConstantRange &Offsets) + Call(uint64_t ParamNo, ValueInfo Callee, const ConstantRange &Offsets) : ParamNo(ParamNo), Callee(Callee), Offsets(Offsets) {} }; diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -472,7 +472,7 @@ F.hasFnAttribute(Attribute::AlwaysInline)}; std::vector ParamAccesses; if (auto *SSI = GetSSICallback(F)) - ParamAccesses = SSI->getParamAccesses(); + ParamAccesses = SSI->getParamAccesses(Index); auto FuncSummary = std::make_unique( Flags, NumInsts, FunFlags, /*EntryCount=*/0, std::move(Refs), CallGraphEdges.takeVector(), TypeTests.takeVector(), diff --git a/llvm/lib/Analysis/StackSafetyAnalysis.cpp b/llvm/lib/Analysis/StackSafetyAnalysis.cpp --- a/llvm/lib/Analysis/StackSafetyAnalysis.cpp +++ b/llvm/lib/Analysis/StackSafetyAnalysis.cpp @@ -22,6 +22,7 @@ #include "llvm/IR/InstIterator.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicInst.h" +#include "llvm/IR/ModuleSummaryIndex.h" #include "llvm/InitializePasses.h" #include "llvm/Support/Casting.h" #include "llvm/Support/CommandLine.h" @@ -785,7 +786,7 @@ } std::vector -StackSafetyInfo::getParamAccesses() const { +StackSafetyInfo::getParamAccesses(ModuleSummaryIndex &Index) const { // Implementation transforms internal representation of parameter information // into FunctionSummary format. std::vector ParamAccesses; @@ -810,7 +811,8 @@ ParamAccesses.pop_back(); break; } - Param.Calls.emplace_back(C.first.ParamNo, C.first.Callee->getGUID(), + Param.Calls.emplace_back(C.first.ParamNo, + Index.getOrInsertValueInfo(C.first.Callee), C.second); llvm::sort(Param.Calls, [](const FunctionSummary::ParamAccess::Call &L, const FunctionSummary::ParamAccess::Call &R) { diff --git a/llvm/lib/AsmParser/LLParser.h b/llvm/lib/AsmParser/LLParser.h --- a/llvm/lib/AsmParser/LLParser.h +++ b/llvm/lib/AsmParser/LLParser.h @@ -372,8 +372,11 @@ bool ParseOptionalParamAccesses( std::vector &Params); bool ParseParamNo(uint64_t &ParamNo); - bool ParseParamAccess(FunctionSummary::ParamAccess &Param); - bool ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call); + using IdLocListType = std::vector>; + bool ParseParamAccess(FunctionSummary::ParamAccess &Param, + IdLocListType &IdLocList); + bool ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call, + IdLocListType &IdLocList); bool ParseParamAccessOffset(ConstantRange &range); bool ParseOptionalRefs(std::vector &Refs); bool ParseTypeIdEntry(unsigned ID); diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -8722,7 +8722,8 @@ /// ParamAccessCall /// := '(' 'callee' ':' GVReference ',' ParamNo ',' ParamAccessOffset ')' -bool LLParser::ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call) { +bool LLParser::ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call, + IdLocListType &IdLocList) { if (ParseToken(lltok::lparen, "expected '(' here") || ParseToken(lltok::kw_callee, "expected 'callee' here") || ParseToken(lltok::colon, "expected ':' here")) @@ -8730,10 +8731,12 @@ unsigned GVId; ValueInfo VI; + LocTy Loc = Lex.getLoc(); if (ParseGVReference(VI, GVId)) return true; - Call.Callee = VI.getGUID(); + Call.Callee = VI; + IdLocList.emplace_back(GVId, Loc); if (ParseToken(lltok::comma, "expected ',' here") || ParseParamNo(Call.ParamNo) || @@ -8750,7 +8753,8 @@ /// ParamAccess /// := '(' ParamNo ',' ParamAccessOffset [',' OptionalParamAccessCalls]? ')' /// OptionalParamAccessCalls := '(' Call [',' Call]* ')' -bool LLParser::ParseParamAccess(FunctionSummary::ParamAccess &Param) { +bool LLParser::ParseParamAccess(FunctionSummary::ParamAccess &Param, + IdLocListType &IdLocList) { if (ParseToken(lltok::lparen, "expected '(' here") || ParseParamNo(Param.ParamNo) || ParseToken(lltok::comma, "expected ',' here") || @@ -8764,7 +8768,7 @@ return true; do { FunctionSummary::ParamAccess::Call Call; - if (ParseParamAccessCall(Call)) + if (ParseParamAccessCall(Call, IdLocList)) return true; Param.Calls.push_back(Call); } while (EatIfPresent(lltok::comma)); @@ -8790,16 +8794,33 @@ ParseToken(lltok::lparen, "expected '(' here")) return true; + IdLocListType VContexts; + size_t CallsNum = 0; do { FunctionSummary::ParamAccess ParamAccess; - if (ParseParamAccess(ParamAccess)) + if (ParseParamAccess(ParamAccess, VContexts)) return true; - Params.push_back(ParamAccess); + CallsNum += ParamAccess.Calls.size(); + assert(VContexts.size() == CallsNum); + Params.emplace_back(std::move(ParamAccess)); } while (EatIfPresent(lltok::comma)); if (ParseToken(lltok::rparen, "expected ')' here")) return true; + // Now that the Params is finalized, it is safe to save the locations + // of any forward GV references that need updating later. + IdLocListType::const_iterator ItContext = VContexts.begin(); + for (auto &PA : Params) { + for (auto &C : PA.Calls) { + if (C.Callee.getRef() == FwdVIRef) + ForwardRefValueInfos[ItContext->first].emplace_back(&C.Callee, + ItContext->second); + ++ItContext; + } + } + assert(ItContext == VContexts.end()); + return false; } diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -835,6 +835,8 @@ void parseTypeIdCompatibleVtableSummaryRecord(ArrayRef Record); void parseTypeIdCompatibleVtableInfo(ArrayRef Record, size_t &Slot, TypeIdCompatibleVtableInfo &TypeId); + std::vector + parseParamAccesses(ArrayRef Record); std::pair getValueInfoFromValueId(unsigned ValueId); @@ -5856,8 +5858,8 @@ parseWholeProgramDevirtResolution(Record, Strtab, Slot, TypeId); } -static std::vector -parseParamAccesses(ArrayRef Record) { +std::vector +ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef Record) { auto ReadRange = [&]() { APInt Lower(FunctionSummary::ParamAccess::RangeWidth, BitcodeReader::decodeSignRotatedValue(Record.front())); @@ -5883,7 +5885,7 @@ for (auto &Call : ParamAccess.Calls) { Call.ParamNo = Record.front(); Record = Record.drop_front(); - Call.Callee = Record.front(); + Call.Callee = getValueInfoFromValueId(Record.front()).first; Record = Record.drop_front(); Call.Offsets = ReadRange(); } diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -3549,8 +3549,10 @@ /// Write the function type metadata related records that need to appear before /// a function summary entry (whether per-module or combined). +template static void writeFunctionTypeMetadataRecords(BitstreamWriter &Stream, - FunctionSummary *FS) { + FunctionSummary *FS, + Fn GetValueID) { if (!FS->type_tests().empty()) Stream.EmitRecord(bitc::FS_TYPE_TESTS, FS->type_tests()); @@ -3600,16 +3602,25 @@ if (!FS->paramAccesses().empty()) { Record.clear(); for (auto &Arg : FS->paramAccesses()) { + size_t UndoSize = Record.size(); Record.push_back(Arg.ParamNo); WriteRange(Arg.Use); Record.push_back(Arg.Calls.size()); for (auto &Call : Arg.Calls) { Record.push_back(Call.ParamNo); - Record.push_back(Call.Callee); + Optional ValueID = GetValueID(Call.Callee); + if (!ValueID) { + // If ValueID is unknown we can't drop just this call, we must drop + // entire parameter. + Record.resize(UndoSize); + break; + } + Record.push_back(*ValueID); WriteRange(Call.Offsets); } } - Stream.EmitRecord(bitc::FS_PARAM_ACCESS, Record); + if (!Record.empty()) + Stream.EmitRecord(bitc::FS_PARAM_ACCESS, Record); } } @@ -3706,7 +3717,11 @@ NameVals.push_back(ValueID); FunctionSummary *FS = cast(Summary); - writeFunctionTypeMetadataRecords(Stream, FS); + + writeFunctionTypeMetadataRecords( + Stream, FS, [&](const ValueInfo &VI) -> Optional { + return {VE.getValueID(VI.getValue())}; + }); auto SpecialRefCnts = FS->specialRefCounts(); NameVals.push_back(getEncodedGVSummaryFlags(FS->flags())); @@ -4083,8 +4098,38 @@ return; } + auto GetValueId = [&](const ValueInfo &VI) -> Optional { + GlobalValue::GUID GUID = VI.getGUID(); + Optional CallValueId = getValueId(GUID); + if (CallValueId) + return CallValueId; + // For SamplePGO, the indirect call targets for local functions will + // have its original name annotated in profile. We try to find the + // corresponding PGOFuncName as the GUID. + GUID = Index.getGUIDFromOriginalID(GUID); + if (!GUID) + return None; + CallValueId = getValueId(GUID); + if (!CallValueId) + return None; + // The mapping from OriginalId to GUID may return a GUID + // that corresponds to a static variable. Filter it out here. + // This can happen when + // 1) There is a call to a library function which does not have + // a CallValidId; + // 2) There is a static variable with the OriginalGUID identical + // to the GUID of the library function in 1); + // When this happens, the logic for SamplePGO kicks in and + // the static variable in 2) will be found, which needs to be + // filtered out. + auto *GVSum = Index.getGlobalValueSummary(GUID, false); + if (GVSum && GVSum->getSummaryKind() == GlobalValueSummary::GlobalVarKind) + return None; + return CallValueId; + }; + auto *FS = cast(S); - writeFunctionTypeMetadataRecords(Stream, FS); + writeFunctionTypeMetadataRecords(Stream, FS, GetValueId); getReferencedTypeIds(FS, ReferencedTypeIds); NameVals.push_back(*ValueId); @@ -4126,33 +4171,9 @@ for (auto &EI : FS->calls()) { // If this GUID doesn't have a value id, it doesn't have a function // summary and we don't need to record any calls to it. - GlobalValue::GUID GUID = EI.first.getGUID(); - auto CallValueId = getValueId(GUID); - if (!CallValueId) { - // For SamplePGO, the indirect call targets for local functions will - // have its original name annotated in profile. We try to find the - // corresponding PGOFuncName as the GUID. - GUID = Index.getGUIDFromOriginalID(GUID); - if (GUID == 0) - continue; - CallValueId = getValueId(GUID); - if (!CallValueId) - continue; - // The mapping from OriginalId to GUID may return a GUID - // that corresponds to a static variable. Filter it out here. - // This can happen when - // 1) There is a call to a library function which does not have - // a CallValidId; - // 2) There is a static variable with the OriginalGUID identical - // to the GUID of the library function in 1); - // When this happens, the logic for SamplePGO kicks in and - // the static variable in 2) will be found, which needs to be - // filtered out. - auto *GVSum = Index.getGlobalValueSummary(GUID, false); - if (GVSum && - GVSum->getSummaryKind() == GlobalValueSummary::GlobalVarKind) - continue; - } + Optional CallValueId = GetValueId(EI.first); + if (!CallValueId) + continue; NameVals.push_back(*CallValueId); if (HasProfileData) NameVals.push_back(static_cast(EI.second.Hotness)); diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp --- a/llvm/lib/IR/AsmWriter.cpp +++ b/llvm/lib/IR/AsmWriter.cpp @@ -3105,7 +3105,7 @@ FieldSeparator IFS; for (auto &Call : PS.Calls) { Out << IFS; - Out << "(callee: ^" << Machine.getGUIDSlot(Call.Callee); + Out << "(callee: ^" << Machine.getGUIDSlot(Call.Callee.getGUID()); Out << ", param: " << Call.ParamNo; Out << ", offset: "; PrintRange(Call.Offsets); diff --git a/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll b/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll --- a/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll +++ b/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll @@ -286,8 +286,8 @@ ; COMBINED: -; COMBINED-NEXT: -; COMBINED-NEXT: +; COMBINED-NEXT: +; COMBINED-NEXT: ; COMBINED-NEXT: ; COMBINED-NEXT: ; COMBINED-NEXT: