Skip to content

[NFC] Coding style fixes: SampleProf #98208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 47 additions & 45 deletions llvm/include/llvm/ProfileData/SampleProf.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ inline std::error_code make_error_code(sampleprof_error E) {
return std::error_code(static_cast<int>(E), sampleprof_category());
}

inline sampleprof_error MergeResult(sampleprof_error &Accumulator,
sampleprof_error Result) {
inline sampleprof_error mergeSampleProfErrors(sampleprof_error &Accumulator,
sampleprof_error Result) {
// Prefer first error encountered as later errors may be secondary effects of
// the initial problem.
if (Accumulator == sampleprof_error::success &&
Expand Down Expand Up @@ -129,7 +129,7 @@ enum SecType {
};

static inline std::string getSecName(SecType Type) {
switch ((int)Type) { // Avoid -Wcovered-switch-default
switch (static_cast<int>(Type)) { // Avoid -Wcovered-switch-default
case SecInValid:
return "InvalidSection";
case SecProfSummary:
Expand Down Expand Up @@ -392,7 +392,7 @@ class SampleRecord {
uint64_t getSamples() const { return NumSamples; }
const CallTargetMap &getCallTargets() const { return CallTargets; }
const SortedCallTargetSet getSortedCallTargets() const {
return SortCallTargets(CallTargets);
return sortCallTargets(CallTargets);
}

uint64_t getCallTargetSum() const {
Expand All @@ -403,7 +403,8 @@ class SampleRecord {
}

/// Sort call targets in descending order of call frequency.
static const SortedCallTargetSet SortCallTargets(const CallTargetMap &Targets) {
static const SortedCallTargetSet
sortCallTargets(const CallTargetMap &Targets) {
SortedCallTargetSet SortedTargets;
for (const auto &[Target, Frequency] : Targets) {
SortedTargets.emplace(Target, Frequency);
Expand Down Expand Up @@ -642,8 +643,8 @@ class SampleContext {
}

/// Set the name of the function and clear the current context.
void setFunction(FunctionId newFunction) {
Func = newFunction;
void setFunction(FunctionId NewFunctionID) {
Func = NewFunctionID;
FullContext = SampleContextFrames();
State = UnknownContext;
}
Expand Down Expand Up @@ -692,7 +693,7 @@ class SampleContext {
}
};

bool IsPrefixOf(const SampleContext &That) const {
bool isPrefixOf(const SampleContext &That) const {
auto ThisContext = FullContext;
auto ThatContext = That.FullContext;
if (ThatContext.size() < ThisContext.size())
Expand Down Expand Up @@ -846,11 +847,11 @@ class FunctionSamples {
}

// Set current context and all callee contexts to be synthetic.
void SetContextSynthetic() {
void setContextSynthetic() {
Context.setState(SyntheticContext);
for (auto &I : CallsiteSamples) {
for (auto &CS : I.second) {
CS.second.SetContextSynthetic();
CS.second.setContextSynthetic();
}
}
}
Expand All @@ -864,32 +865,31 @@ class FunctionSamples {
const auto &ProfileLoc = IRToProfileLocationMap->find(IRLoc);
if (ProfileLoc != IRToProfileLocationMap->end())
return ProfileLoc->second;
else
return IRLoc;
return IRLoc;
}

/// Return the number of samples collected at the given location.
/// Each location is specified by \p LineOffset and \p Discriminator.
/// If the location is not found in profile, return error.
ErrorOr<uint64_t> findSamplesAt(uint32_t LineOffset,
uint32_t Discriminator) const {
const auto &ret = BodySamples.find(
const auto &Ret = BodySamples.find(
mapIRLocToProfileLoc(LineLocation(LineOffset, Discriminator)));
if (ret == BodySamples.end())
if (Ret == BodySamples.end())
return std::error_code();
return ret->second.getSamples();
return Ret->second.getSamples();
}

/// Returns the call target map collected at a given location.
/// Each location is specified by \p LineOffset and \p Discriminator.
/// If the location is not found in profile, return error.
ErrorOr<const SampleRecord::CallTargetMap &>
findCallTargetMapAt(uint32_t LineOffset, uint32_t Discriminator) const {
const auto &ret = BodySamples.find(
const auto &Ret = BodySamples.find(
mapIRLocToProfileLoc(LineLocation(LineOffset, Discriminator)));
if (ret == BodySamples.end())
if (Ret == BodySamples.end())
return std::error_code();
return ret->second.getCallTargets();
return Ret->second.getCallTargets();
}

/// Returns the call target map collected at a given location specified by \p
Expand All @@ -910,10 +910,10 @@ class FunctionSamples {
/// Returns the FunctionSamplesMap at the given \p Loc.
const FunctionSamplesMap *
findFunctionSamplesMapAt(const LineLocation &Loc) const {
auto iter = CallsiteSamples.find(mapIRLocToProfileLoc(Loc));
if (iter == CallsiteSamples.end())
auto Iter = CallsiteSamples.find(mapIRLocToProfileLoc(Loc));
if (Iter == CallsiteSamples.end())
return nullptr;
return &iter->second;
return &Iter->second;
}

/// Returns a pointer to FunctionSamples at the given callsite location
Expand Down Expand Up @@ -960,8 +960,8 @@ class FunctionSamples {
else if (!CallsiteSamples.empty()) {
// An indirect callsite may be promoted to several inlined direct calls.
// We need to get the sum of them.
for (const auto &N_FS : CallsiteSamples.begin()->second)
Count += N_FS.second.getHeadSamplesEstimate();
for (const auto &FuncSamples : CallsiteSamples.begin()->second)
Count += FuncSamples.second.getHeadSamplesEstimate();
}
// Return at least 1 if total sample is not 0.
return Count ? Count : TotalSamples > 0;
Expand Down Expand Up @@ -1013,18 +1013,21 @@ class FunctionSamples {
return sampleprof_error::hash_mismatch;
}

MergeResult(Result, addTotalSamples(Other.getTotalSamples(), Weight));
MergeResult(Result, addHeadSamples(Other.getHeadSamples(), Weight));
mergeSampleProfErrors(Result,
addTotalSamples(Other.getTotalSamples(), Weight));
mergeSampleProfErrors(Result,
addHeadSamples(Other.getHeadSamples(), Weight));
for (const auto &I : Other.getBodySamples()) {
const LineLocation &Loc = I.first;
const SampleRecord &Rec = I.second;
MergeResult(Result, BodySamples[Loc].merge(Rec, Weight));
mergeSampleProfErrors(Result, BodySamples[Loc].merge(Rec, Weight));
}
for (const auto &I : Other.getCallsiteSamples()) {
const LineLocation &Loc = I.first;
FunctionSamplesMap &FSMap = functionSamplesAt(Loc);
for (const auto &Rec : I.second)
MergeResult(Result, FSMap[Rec.first].merge(Rec.second, Weight));
mergeSampleProfErrors(Result,
FSMap[Rec.first].merge(Rec.second, Weight));
}
return Result;
}
Expand All @@ -1039,10 +1042,10 @@ class FunctionSamples {
uint64_t Threshold) const {
if (TotalSamples <= Threshold)
return;
auto isDeclaration = [](const Function *F) {
auto IsDeclaration = [](const Function *F) {
return !F || F->isDeclaration();
};
if (isDeclaration(SymbolMap.lookup(getFunction()))) {
if (IsDeclaration(SymbolMap.lookup(getFunction()))) {
// Add to the import list only when it's defined out of module.
S.insert(getGUID());
}
Expand All @@ -1052,7 +1055,7 @@ class FunctionSamples {
for (const auto &TS : BS.second.getCallTargets())
if (TS.second > Threshold) {
const Function *Callee = SymbolMap.lookup(TS.first);
if (isDeclaration(Callee))
if (IsDeclaration(Callee))
S.insert(TS.first.getHashCode());
}
for (const auto &CS : CallsiteSamples)
Expand All @@ -1061,8 +1064,8 @@ class FunctionSamples {
}

/// Set the name of the function.
void setFunction(FunctionId newFunction) {
Context.setFunction(newFunction);
void setFunction(FunctionId NewFunctionID) {
Context.setFunction(NewFunctionID);
}

/// Return the function name.
Expand All @@ -1083,7 +1086,7 @@ class FunctionSamples {
/// Return the canonical name for a function, taking into account
/// suffix elision policy attributes.
static StringRef getCanonicalFnName(const Function &F) {
auto AttrName = "sample-profile-suffix-elision-policy";
const char *AttrName = "sample-profile-suffix-elision-policy";
auto Attr = F.getFnAttribute(AttrName).getValueAsString();
return getCanonicalFnName(F.getName(), Attr);
}
Expand All @@ -1099,12 +1102,12 @@ class FunctionSamples {
// Note the sequence of the suffixes in the knownSuffixes array matters.
// If suffix "A" is appended after the suffix "B", "A" should be in front
// of "B" in knownSuffixes.
const char *knownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
if (Attr == "" || Attr == "all") {
const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
if (Attr == "" || Attr == "all")
return FnName.split('.').first;
} else if (Attr == "selected") {
if (Attr == "selected") {
StringRef Cand(FnName);
for (const auto &Suf : knownSuffixes) {
for (const auto &Suf : KnownSuffixes) {
StringRef Suffix(Suf);
// If the profile contains ".__uniq." suffix, don't strip the
// suffix for names in the IR.
Expand All @@ -1118,11 +1121,10 @@ class FunctionSamples {
Cand = Cand.substr(0, It);
}
return Cand;
} else if (Attr == "none") {
return FnName;
} else {
assert(false && "internal error: unknown suffix elision policy");
}
if (Attr == "none")
return FnName;
assert(false && "internal error: unknown suffix elision policy");
return FnName;
}

Expand Down Expand Up @@ -1307,7 +1309,7 @@ class SampleProfileMap
public:
// Convenience method because this is being used in many places. Set the
// FunctionSamples' context if its newly inserted.
mapped_type &Create(const SampleContext &Ctx) {
mapped_type &create(const SampleContext &Ctx) {
auto Ret = try_emplace(Ctx, FunctionSamples());
if (Ret.second)
Ret.first->second.setContext(Ctx);
Expand Down Expand Up @@ -1428,7 +1430,7 @@ class ProfileConverter {
for (const auto &I : InputProfiles) {
// Retain the profile name and clear the full context for each function
// profile.
FunctionSamples &FS = OutputProfiles.Create(I.second.getFunction());
FunctionSamples &FS = OutputProfiles.create(I.second.getFunction());
FS.merge(I.second);
}
} else {
Expand Down Expand Up @@ -1507,8 +1509,8 @@ class ProfileSymbolList {
public:
/// copy indicates whether we need to copy the underlying memory
/// for the input Name.
void add(StringRef Name, bool copy = false) {
if (!copy) {
void add(StringRef Name, bool Copy = false) {
if (!Copy) {
Syms.insert(Name);
return;
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/ProfileData/SampleProf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ sampleprof_error SampleRecord::merge(const SampleRecord &Other,
sampleprof_error Result;
Result = addSamples(Other.getSamples(), Weight);
for (const auto &I : Other.getCallTargets()) {
MergeResult(Result, addCalledTarget(I.first, I.second, Weight));
mergeSampleProfErrors(Result, addCalledTarget(I.first, I.second, Weight));
}
return Result;
}
Expand Down Expand Up @@ -364,7 +364,7 @@ void SampleContextTrimmer::trimAndMergeColdContextProfiles(
if (ColdContextFrameLength < MergedContext.size())
MergedContext = MergedContext.take_back(ColdContextFrameLength);
// Need to set MergedProfile's context here otherwise it will be lost.
FunctionSamples &MergedProfile = MergedProfileMap.Create(MergedContext);
FunctionSamples &MergedProfile = MergedProfileMap.create(MergedContext);
MergedProfile.merge(*I.second);
}
ProfileMap.erase(I.first);
Expand Down
25 changes: 13 additions & 12 deletions llvm/lib/ProfileData/SampleProfReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,9 @@ std::error_code SampleProfileReaderText::readImpl() {
SampleContext FContext(FName, CSNameTable);
if (FContext.hasContext())
++CSProfileCount;
FunctionSamples &FProfile = Profiles.Create(FContext);
MergeResult(Result, FProfile.addTotalSamples(NumSamples));
MergeResult(Result, FProfile.addHeadSamples(NumHeadSamples));
FunctionSamples &FProfile = Profiles.create(FContext);
mergeSampleProfErrors(Result, FProfile.addTotalSamples(NumSamples));
mergeSampleProfErrors(Result, FProfile.addHeadSamples(NumHeadSamples));
InlineStack.clear();
InlineStack.push_back(&FProfile);
} else {
Expand Down Expand Up @@ -394,7 +394,7 @@ std::error_code SampleProfileReaderText::readImpl() {
FunctionSamples &FSamples = InlineStack.back()->functionSamplesAt(
LineLocation(LineOffset, Discriminator))[FunctionId(FName)];
FSamples.setFunction(FunctionId(FName));
MergeResult(Result, FSamples.addTotalSamples(NumSamples));
mergeSampleProfErrors(Result, FSamples.addTotalSamples(NumSamples));
InlineStack.push_back(&FSamples);
DepthMetadata = 0;
break;
Expand All @@ -405,13 +405,14 @@ std::error_code SampleProfileReaderText::readImpl() {
}
FunctionSamples &FProfile = *InlineStack.back();
for (const auto &name_count : TargetCountMap) {
MergeResult(Result, FProfile.addCalledTargetSamples(
LineOffset, Discriminator,
FunctionId(name_count.first),
name_count.second));
mergeSampleProfErrors(Result, FProfile.addCalledTargetSamples(
LineOffset, Discriminator,
FunctionId(name_count.first),
name_count.second));
}
MergeResult(Result, FProfile.addBodySamples(LineOffset, Discriminator,
NumSamples));
mergeSampleProfErrors(
Result,
FProfile.addBodySamples(LineOffset, Discriminator, NumSamples));
break;
}
case LineType::Metadata: {
Expand Down Expand Up @@ -892,12 +893,12 @@ std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
if ((useMD5() && FuncGuidsToUse.count(FName.getHashCode())) ||
(!useMD5() && (FuncsToUse.count(FNameString) ||
(Remapper && Remapper->exist(FNameString))))) {
if (!CommonContext || !CommonContext->IsPrefixOf(FContext))
if (!CommonContext || !CommonContext->isPrefixOf(FContext))
CommonContext = &FContext;
}

if (CommonContext == &FContext ||
(CommonContext && CommonContext->IsPrefixOf(FContext))) {
(CommonContext && CommonContext->isPrefixOf(FContext))) {
// Load profile for the current context which originated from
// the common ancestor.
const uint8_t *FuncProfileAddr = Start + NameOffset.second;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/IPO/SampleContextTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ void SampleContextTracker::createContextLessProfileMap(
FunctionSamples *FProfile = Node->getFunctionSamples();
// Profile's context can be empty, use ContextNode's func name.
if (FProfile)
ContextLessProfiles.Create(Node->getFuncName()).merge(*FProfile);
ContextLessProfiles.create(Node->getFuncName()).merge(*FProfile);
}
}
} // namespace llvm
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/IPO/SampleProfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
FunctionId(FunctionSamples::getCanonicalFnName(Callee->getName()))];
OutlineFS->merge(*FS, 1);
// Set outlined profile to be synthetic to not bias the inliner.
OutlineFS->SetContextSynthetic();
OutlineFS->setContextSynthetic();
}
} else {
auto pair =
Expand All @@ -1586,7 +1586,7 @@ void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
static SmallVector<InstrProfValueData, 2>
GetSortedValueDataFromCallTargets(const SampleRecord::CallTargetMap &M) {
SmallVector<InstrProfValueData, 2> R;
for (const auto &I : SampleRecord::SortCallTargets(M)) {
for (const auto &I : SampleRecord::sortCallTargets(M)) {
R.emplace_back(
InstrProfValueData{I.first.getHashCode(), I.second});
}
Expand Down
6 changes: 4 additions & 2 deletions llvm/tools/llvm-profdata/llvm-profdata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,8 @@ remapSamples(const sampleprof::FunctionSamples &Samples,
for (const auto &Callsite : CallsiteSamples.second) {
sampleprof::FunctionSamples Remapped =
remapSamples(Callsite.second, Remapper, Error);
MergeResult(Error, Target[Remapped.getFunction()].merge(Remapped));
mergeSampleProfErrors(Error,
Target[Remapped.getFunction()].merge(Remapped));
}
}
return Result;
Expand Down Expand Up @@ -1542,7 +1543,8 @@ static void mergeSampleProfile(const WeightedFileVector &Inputs,
: FunctionSamples();
FunctionSamples &Samples = Remapper ? Remapped : I->second;
SampleContext FContext = Samples.getContext();
MergeResult(Result, ProfileMap[FContext].merge(Samples, Input.Weight));
mergeSampleProfErrors(Result,
ProfileMap[FContext].merge(Samples, Input.Weight));
if (Result != sampleprof_error::success) {
std::error_code EC = make_error_code(Result);
handleMergeWriterError(errorCodeToError(EC), Input.Filename,
Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/llvm-profgen/ProfileGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ bool CSProfileGenerator::collectFunctionsFromLLVMProfile(
FunctionSamples &
ProfileGenerator::getTopLevelFunctionProfile(FunctionId FuncName) {
SampleContext Context(FuncName);
return ProfileMap.Create(Context);
return ProfileMap.create(Context);
}

void ProfileGenerator::generateProfile() {
Expand Down
Loading