[llvm] [NFC] Coding style fixes: SampleProf (PR #98208)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 9 12:49:39 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-pgo
Author: Mircea Trofin (mtrofin)
<details>
<summary>Changes</summary>
Also some control flow simplifications.
Notably, this doesn't address `sampleprof_error`. I *think* the style there tries to match `std::error_category`.
Also left `hash_value` as-is, because it matches what we do in Hashing.h
---
Full diff: https://github.com/llvm/llvm-project/pull/98208.diff
5 Files Affected:
- (modified) llvm/include/llvm/ProfileData/SampleProf.h (+47-45)
- (modified) llvm/lib/ProfileData/SampleProf.cpp (+2-2)
- (modified) llvm/lib/ProfileData/SampleProfReader.cpp (+13-12)
- (modified) llvm/lib/Transforms/IPO/SampleProfile.cpp (+2-2)
- (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+4-2)
``````````diff
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index 51d590be124f1..be21f8497bdac 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -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 &&
@@ -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:
@@ -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 {
@@ -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);
@@ -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 NewFunction) {
+ Func = NewFunction;
FullContext = SampleContextFrames();
State = UnknownContext;
}
@@ -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())
@@ -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();
}
}
}
@@ -864,8 +865,7 @@ 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.
@@ -873,11 +873,11 @@ class FunctionSamples {
/// 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.
@@ -885,11 +885,11 @@ class FunctionSamples {
/// 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
@@ -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
@@ -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;
@@ -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;
}
@@ -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());
}
@@ -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)
@@ -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.
@@ -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);
}
@@ -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.
@@ -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;
}
@@ -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);
@@ -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 {
@@ -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;
}
diff --git a/llvm/lib/ProfileData/SampleProf.cpp b/llvm/lib/ProfileData/SampleProf.cpp
index 59fa71899ed47..294f64636d989 100644
--- a/llvm/lib/ProfileData/SampleProf.cpp
+++ b/llvm/lib/ProfileData/SampleProf.cpp
@@ -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;
}
@@ -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);
diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index f91a0e6177ea0..79155f4b1d29a 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -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 {
@@ -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;
@@ -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: {
@@ -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;
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 0b3a6931e779b..79a5a1779f579 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1581,7 +1581,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 =
@@ -1595,7 +1595,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});
}
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 78daf9f7dc109..49268d36e4749 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -1403,7 +1403,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;
@@ -1524,7 +1525,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,
``````````
</details>
https://github.com/llvm/llvm-project/pull/98208
More information about the llvm-commits
mailing list