[llvm] fc4fd89 - [StackSafety] Use ValueInfo in ParamAccess::Call
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 14 12:43:13 PDT 2020
Author: Vitaly Buka
Date: 2020-08-14T12:42:44-07:00
New Revision: fc4fd898522afac2d54003f34a4656d898358b03
URL: https://github.com/llvm/llvm-project/commit/fc4fd898522afac2d54003f34a4656d898358b03
DIFF: https://github.com/llvm/llvm-project/commit/fc4fd898522afac2d54003f34a4656d898358b03.diff
LOG: [StackSafety] Use ValueInfo in ParamAccess::Call
This avoid GUID lookup in Index.findSummaryInModule.
Follow up for D81242.
Reviewed By: tejohnson
Differential Revision: https://reviews.llvm.org/D85269
Added:
Modified:
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/AsmParser/LLParser.h
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
index 846c2e6f7e91..59c1e3e3bd56 100644
--- a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
+++ b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
@@ -51,7 +51,8 @@ class StackSafetyInfo {
/// 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<FunctionSummary::ParamAccess> getParamAccesses() const;
+ std::vector<FunctionSummary::ParamAccess>
+ getParamAccesses(ModuleSummaryIndex &Index) const;
};
class StackSafetyGlobalInfo {
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index e78892cf83eb..574ade6a7a11 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -562,12 +562,11 @@ class FunctionSummary : public GlobalValueSummary {
/// 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
index e7d529d0b51e..7d06aac08619 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -472,7 +472,7 @@ static void computeFunctionSummary(
F.hasFnAttribute(Attribute::AlwaysInline)};
std::vector<FunctionSummary::ParamAccess> ParamAccesses;
if (auto *SSI = GetSSICallback(F))
- ParamAccesses = SSI->getParamAccesses();
+ ParamAccesses = SSI->getParamAccesses(Index);
auto FuncSummary = std::make_unique<FunctionSummary>(
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
index cd74a0250457..f61363909c5b 100644
--- 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 @@ const StackSafetyGlobalInfo::InfoTy &StackSafetyGlobalInfo::getInfo() const {
}
std::vector<FunctionSummary::ParamAccess>
-StackSafetyInfo::getParamAccesses() const {
+StackSafetyInfo::getParamAccesses(ModuleSummaryIndex &Index) const {
// Implementation transforms internal representation of parameter information
// into FunctionSummary format.
std::vector<FunctionSummary::ParamAccess> ParamAccesses;
@@ -810,7 +811,8 @@ StackSafetyInfo::getParamAccesses() const {
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.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 1fd7429aae41..06e5709983ea 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -8722,7 +8722,8 @@ bool LLParser::ParseParamAccessOffset(ConstantRange &Range) {
/// 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 @@ bool LLParser::ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call) {
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 @@ bool LLParser::ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call) {
/// 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 @@ bool LLParser::ParseParamAccess(FunctionSummary::ParamAccess &Param) {
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 @@ bool LLParser::ParseOptionalParamAccesses(
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/AsmParser/LLParser.h b/llvm/lib/AsmParser/LLParser.h
index 333b8a35cfce..a7fbcdd5abc5 100644
--- a/llvm/lib/AsmParser/LLParser.h
+++ b/llvm/lib/AsmParser/LLParser.h
@@ -372,8 +372,11 @@ namespace llvm {
bool ParseOptionalParamAccesses(
std::vector<FunctionSummary::ParamAccess> &Params);
bool ParseParamNo(uint64_t &ParamNo);
- bool ParseParamAccess(FunctionSummary::ParamAccess &Param);
- bool ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call);
+ using IdLocListType = std::vector<std::pair<unsigned, LocTy>>;
+ bool ParseParamAccess(FunctionSummary::ParamAccess &Param,
+ IdLocListType &IdLocList);
+ bool ParseParamAccessCall(FunctionSummary::ParamAccess::Call &Call,
+ IdLocListType &IdLocList);
bool ParseParamAccessOffset(ConstantRange &range);
bool ParseOptionalRefs(std::vector<ValueInfo> &Refs);
bool ParseTypeIdEntry(unsigned ID);
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 82b6f2078695..0fa502f4569f 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -835,6 +835,8 @@ class ModuleSummaryIndexBitcodeReader : public BitcodeReaderBase {
void parseTypeIdCompatibleVtableSummaryRecord(ArrayRef<uint64_t> Record);
void parseTypeIdCompatibleVtableInfo(ArrayRef<uint64_t> Record, size_t &Slot,
TypeIdCompatibleVtableInfo &TypeId);
+ std::vector<FunctionSummary::ParamAccess>
+ parseParamAccesses(ArrayRef<uint64_t> Record);
std::pair<ValueInfo, GlobalValue::GUID>
getValueInfoFromValueId(unsigned ValueId);
@@ -5856,8 +5858,8 @@ static void parseTypeIdSummaryRecord(ArrayRef<uint64_t> Record,
parseWholeProgramDevirtResolution(Record, Strtab, Slot, TypeId);
}
-static std::vector<FunctionSummary::ParamAccess>
-parseParamAccesses(ArrayRef<uint64_t> Record) {
+std::vector<FunctionSummary::ParamAccess>
+ModuleSummaryIndexBitcodeReader::parseParamAccesses(ArrayRef<uint64_t> Record) {
auto ReadRange = [&]() {
APInt Lower(FunctionSummary::ParamAccess::RangeWidth,
BitcodeReader::decodeSignRotatedValue(Record.front()));
@@ -5883,7 +5885,7 @@ parseParamAccesses(ArrayRef<uint64_t> Record) {
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
index b6b2eb3b0dbe..777a0e2ba129 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -3549,8 +3549,10 @@ void IndexBitcodeWriter::writeModStrings() {
/// Write the function type metadata related records that need to appear before
/// a function summary entry (whether per-module or combined).
+template <typename Fn>
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 @@ static void writeFunctionTypeMetadataRecords(BitstreamWriter &Stream,
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<unsigned> 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 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
NameVals.push_back(ValueID);
FunctionSummary *FS = cast<FunctionSummary>(Summary);
- writeFunctionTypeMetadataRecords(Stream, FS);
+
+ writeFunctionTypeMetadataRecords(
+ Stream, FS, [&](const ValueInfo &VI) -> Optional<unsigned> {
+ return {VE.getValueID(VI.getValue())};
+ });
auto SpecialRefCnts = FS->specialRefCounts();
NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
@@ -4083,8 +4098,38 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
return;
}
+ auto GetValueId = [&](const ValueInfo &VI) -> Optional<unsigned> {
+ GlobalValue::GUID GUID = VI.getGUID();
+ Optional<unsigned> 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<FunctionSummary>(S);
- writeFunctionTypeMetadataRecords(Stream, FS);
+ writeFunctionTypeMetadataRecords(Stream, FS, GetValueId);
getReferencedTypeIds(FS, ReferencedTypeIds);
NameVals.push_back(*ValueId);
@@ -4126,33 +4171,9 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
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<unsigned> CallValueId = GetValueId(EI.first);
+ if (!CallValueId)
+ continue;
NameVals.push_back(*CallValueId);
if (HasProfileData)
NameVals.push_back(static_cast<uint8_t>(EI.second.Hotness));
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 5073ee9eb3d2..5e4b717f92c6 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -3105,7 +3105,7 @@ void AssemblyWriter::printFunctionSummary(const FunctionSummary *FS) {
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
index 1757173ec52b..3b53e73ce1d4 100644
--- a/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll
+++ b/llvm/test/Bitcode/thinlto-function-summary-paramaccess.ll
@@ -286,8 +286,8 @@ entry:
; COMBINED: <FLAGS op0=0/>
-; COMBINED-NEXT: <VALUE_GUID op0=1 op1=[[CALLEE1:72710208629861106]]/>
-; COMBINED-NEXT: <VALUE_GUID op0=2 op1=[[CALLEE2:900789920918863816]]/>
+; COMBINED-NEXT: <VALUE_GUID op0=[[CALLEE1:1]] op1=72710208629861106/>
+; COMBINED-NEXT: <VALUE_GUID op0=[[CALLEE2:2]] op1=900789920918863816/>
; COMBINED-NEXT: <VALUE_GUID op0=3 op1=1075564720951610524/>
; COMBINED-NEXT: <VALUE_GUID op0=4 op1=1417835201204712148/>
; COMBINED-NEXT: <VALUE_GUID op0=5 op1=2949024673554120799/>
More information about the llvm-commits
mailing list