Skip to content

Commit 4b897ab

Browse files
mtrofinaaryanshukla
authored andcommitted
[NFC] Coding style fixes: SampleProf (llvm#98208)
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
1 parent 05d1e33 commit 4b897ab

File tree

7 files changed

+70
-65
lines changed

7 files changed

+70
-65
lines changed

llvm/include/llvm/ProfileData/SampleProf.h

Lines changed: 47 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ inline std::error_code make_error_code(sampleprof_error E) {
6666
return std::error_code(static_cast<int>(E), sampleprof_category());
6767
}
6868

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

131131
static inline std::string getSecName(SecType Type) {
132-
switch ((int)Type) { // Avoid -Wcovered-switch-default
132+
switch (static_cast<int>(Type)) { // Avoid -Wcovered-switch-default
133133
case SecInValid:
134134
return "InvalidSection";
135135
case SecProfSummary:
@@ -392,7 +392,7 @@ class SampleRecord {
392392
uint64_t getSamples() const { return NumSamples; }
393393
const CallTargetMap &getCallTargets() const { return CallTargets; }
394394
const SortedCallTargetSet getSortedCallTargets() const {
395-
return SortCallTargets(CallTargets);
395+
return sortCallTargets(CallTargets);
396396
}
397397

398398
uint64_t getCallTargetSum() const {
@@ -403,7 +403,8 @@ class SampleRecord {
403403
}
404404

405405
/// Sort call targets in descending order of call frequency.
406-
static const SortedCallTargetSet SortCallTargets(const CallTargetMap &Targets) {
406+
static const SortedCallTargetSet
407+
sortCallTargets(const CallTargetMap &Targets) {
407408
SortedCallTargetSet SortedTargets;
408409
for (const auto &[Target, Frequency] : Targets) {
409410
SortedTargets.emplace(Target, Frequency);
@@ -642,8 +643,8 @@ class SampleContext {
642643
}
643644

644645
/// Set the name of the function and clear the current context.
645-
void setFunction(FunctionId newFunction) {
646-
Func = newFunction;
646+
void setFunction(FunctionId NewFunctionID) {
647+
Func = NewFunctionID;
647648
FullContext = SampleContextFrames();
648649
State = UnknownContext;
649650
}
@@ -692,7 +693,7 @@ class SampleContext {
692693
}
693694
};
694695

695-
bool IsPrefixOf(const SampleContext &That) const {
696+
bool isPrefixOf(const SampleContext &That) const {
696697
auto ThisContext = FullContext;
697698
auto ThatContext = That.FullContext;
698699
if (ThatContext.size() < ThisContext.size())
@@ -846,11 +847,11 @@ class FunctionSamples {
846847
}
847848

848849
// Set current context and all callee contexts to be synthetic.
849-
void SetContextSynthetic() {
850+
void setContextSynthetic() {
850851
Context.setState(SyntheticContext);
851852
for (auto &I : CallsiteSamples) {
852853
for (auto &CS : I.second) {
853-
CS.second.SetContextSynthetic();
854+
CS.second.setContextSynthetic();
854855
}
855856
}
856857
}
@@ -864,32 +865,31 @@ class FunctionSamples {
864865
const auto &ProfileLoc = IRToProfileLocationMap->find(IRLoc);
865866
if (ProfileLoc != IRToProfileLocationMap->end())
866867
return ProfileLoc->second;
867-
else
868-
return IRLoc;
868+
return IRLoc;
869869
}
870870

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

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

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

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

1016-
MergeResult(Result, addTotalSamples(Other.getTotalSamples(), Weight));
1017-
MergeResult(Result, addHeadSamples(Other.getHeadSamples(), Weight));
1016+
mergeSampleProfErrors(Result,
1017+
addTotalSamples(Other.getTotalSamples(), Weight));
1018+
mergeSampleProfErrors(Result,
1019+
addHeadSamples(Other.getHeadSamples(), Weight));
10181020
for (const auto &I : Other.getBodySamples()) {
10191021
const LineLocation &Loc = I.first;
10201022
const SampleRecord &Rec = I.second;
1021-
MergeResult(Result, BodySamples[Loc].merge(Rec, Weight));
1023+
mergeSampleProfErrors(Result, BodySamples[Loc].merge(Rec, Weight));
10221024
}
10231025
for (const auto &I : Other.getCallsiteSamples()) {
10241026
const LineLocation &Loc = I.first;
10251027
FunctionSamplesMap &FSMap = functionSamplesAt(Loc);
10261028
for (const auto &Rec : I.second)
1027-
MergeResult(Result, FSMap[Rec.first].merge(Rec.second, Weight));
1029+
mergeSampleProfErrors(Result,
1030+
FSMap[Rec.first].merge(Rec.second, Weight));
10281031
}
10291032
return Result;
10301033
}
@@ -1039,10 +1042,10 @@ class FunctionSamples {
10391042
uint64_t Threshold) const {
10401043
if (TotalSamples <= Threshold)
10411044
return;
1042-
auto isDeclaration = [](const Function *F) {
1045+
auto IsDeclaration = [](const Function *F) {
10431046
return !F || F->isDeclaration();
10441047
};
1045-
if (isDeclaration(SymbolMap.lookup(getFunction()))) {
1048+
if (IsDeclaration(SymbolMap.lookup(getFunction()))) {
10461049
// Add to the import list only when it's defined out of module.
10471050
S.insert(getGUID());
10481051
}
@@ -1052,7 +1055,7 @@ class FunctionSamples {
10521055
for (const auto &TS : BS.second.getCallTargets())
10531056
if (TS.second > Threshold) {
10541057
const Function *Callee = SymbolMap.lookup(TS.first);
1055-
if (isDeclaration(Callee))
1058+
if (IsDeclaration(Callee))
10561059
S.insert(TS.first.getHashCode());
10571060
}
10581061
for (const auto &CS : CallsiteSamples)
@@ -1061,8 +1064,8 @@ class FunctionSamples {
10611064
}
10621065

10631066
/// Set the name of the function.
1064-
void setFunction(FunctionId newFunction) {
1065-
Context.setFunction(newFunction);
1067+
void setFunction(FunctionId NewFunctionID) {
1068+
Context.setFunction(NewFunctionID);
10661069
}
10671070

10681071
/// Return the function name.
@@ -1083,7 +1086,7 @@ class FunctionSamples {
10831086
/// Return the canonical name for a function, taking into account
10841087
/// suffix elision policy attributes.
10851088
static StringRef getCanonicalFnName(const Function &F) {
1086-
auto AttrName = "sample-profile-suffix-elision-policy";
1089+
const char *AttrName = "sample-profile-suffix-elision-policy";
10871090
auto Attr = F.getFnAttribute(AttrName).getValueAsString();
10881091
return getCanonicalFnName(F.getName(), Attr);
10891092
}
@@ -1099,12 +1102,12 @@ class FunctionSamples {
10991102
// Note the sequence of the suffixes in the knownSuffixes array matters.
11001103
// If suffix "A" is appended after the suffix "B", "A" should be in front
11011104
// of "B" in knownSuffixes.
1102-
const char *knownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
1103-
if (Attr == "" || Attr == "all") {
1105+
const char *KnownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
1106+
if (Attr == "" || Attr == "all")
11041107
return FnName.split('.').first;
1105-
} else if (Attr == "selected") {
1108+
if (Attr == "selected") {
11061109
StringRef Cand(FnName);
1107-
for (const auto &Suf : knownSuffixes) {
1110+
for (const auto &Suf : KnownSuffixes) {
11081111
StringRef Suffix(Suf);
11091112
// If the profile contains ".__uniq." suffix, don't strip the
11101113
// suffix for names in the IR.
@@ -1118,11 +1121,10 @@ class FunctionSamples {
11181121
Cand = Cand.substr(0, It);
11191122
}
11201123
return Cand;
1121-
} else if (Attr == "none") {
1122-
return FnName;
1123-
} else {
1124-
assert(false && "internal error: unknown suffix elision policy");
11251124
}
1125+
if (Attr == "none")
1126+
return FnName;
1127+
assert(false && "internal error: unknown suffix elision policy");
11261128
return FnName;
11271129
}
11281130

@@ -1307,7 +1309,7 @@ class SampleProfileMap
13071309
public:
13081310
// Convenience method because this is being used in many places. Set the
13091311
// FunctionSamples' context if its newly inserted.
1310-
mapped_type &Create(const SampleContext &Ctx) {
1312+
mapped_type &create(const SampleContext &Ctx) {
13111313
auto Ret = try_emplace(Ctx, FunctionSamples());
13121314
if (Ret.second)
13131315
Ret.first->second.setContext(Ctx);
@@ -1428,7 +1430,7 @@ class ProfileConverter {
14281430
for (const auto &I : InputProfiles) {
14291431
// Retain the profile name and clear the full context for each function
14301432
// profile.
1431-
FunctionSamples &FS = OutputProfiles.Create(I.second.getFunction());
1433+
FunctionSamples &FS = OutputProfiles.create(I.second.getFunction());
14321434
FS.merge(I.second);
14331435
}
14341436
} else {
@@ -1507,8 +1509,8 @@ class ProfileSymbolList {
15071509
public:
15081510
/// copy indicates whether we need to copy the underlying memory
15091511
/// for the input Name.
1510-
void add(StringRef Name, bool copy = false) {
1511-
if (!copy) {
1512+
void add(StringRef Name, bool Copy = false) {
1513+
if (!Copy) {
15121514
Syms.insert(Name);
15131515
return;
15141516
}

llvm/lib/ProfileData/SampleProf.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ sampleprof_error SampleRecord::merge(const SampleRecord &Other,
121121
sampleprof_error Result;
122122
Result = addSamples(Other.getSamples(), Weight);
123123
for (const auto &I : Other.getCallTargets()) {
124-
MergeResult(Result, addCalledTarget(I.first, I.second, Weight));
124+
mergeSampleProfErrors(Result, addCalledTarget(I.first, I.second, Weight));
125125
}
126126
return Result;
127127
}
@@ -364,7 +364,7 @@ void SampleContextTrimmer::trimAndMergeColdContextProfiles(
364364
if (ColdContextFrameLength < MergedContext.size())
365365
MergedContext = MergedContext.take_back(ColdContextFrameLength);
366366
// Need to set MergedProfile's context here otherwise it will be lost.
367-
FunctionSamples &MergedProfile = MergedProfileMap.Create(MergedContext);
367+
FunctionSamples &MergedProfile = MergedProfileMap.create(MergedContext);
368368
MergedProfile.merge(*I.second);
369369
}
370370
ProfileMap.erase(I.first);

llvm/lib/ProfileData/SampleProfReader.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,9 @@ std::error_code SampleProfileReaderText::readImpl() {
355355
SampleContext FContext(FName, CSNameTable);
356356
if (FContext.hasContext())
357357
++CSProfileCount;
358-
FunctionSamples &FProfile = Profiles.Create(FContext);
359-
MergeResult(Result, FProfile.addTotalSamples(NumSamples));
360-
MergeResult(Result, FProfile.addHeadSamples(NumHeadSamples));
358+
FunctionSamples &FProfile = Profiles.create(FContext);
359+
mergeSampleProfErrors(Result, FProfile.addTotalSamples(NumSamples));
360+
mergeSampleProfErrors(Result, FProfile.addHeadSamples(NumHeadSamples));
361361
InlineStack.clear();
362362
InlineStack.push_back(&FProfile);
363363
} else {
@@ -394,7 +394,7 @@ std::error_code SampleProfileReaderText::readImpl() {
394394
FunctionSamples &FSamples = InlineStack.back()->functionSamplesAt(
395395
LineLocation(LineOffset, Discriminator))[FunctionId(FName)];
396396
FSamples.setFunction(FunctionId(FName));
397-
MergeResult(Result, FSamples.addTotalSamples(NumSamples));
397+
mergeSampleProfErrors(Result, FSamples.addTotalSamples(NumSamples));
398398
InlineStack.push_back(&FSamples);
399399
DepthMetadata = 0;
400400
break;
@@ -405,13 +405,14 @@ std::error_code SampleProfileReaderText::readImpl() {
405405
}
406406
FunctionSamples &FProfile = *InlineStack.back();
407407
for (const auto &name_count : TargetCountMap) {
408-
MergeResult(Result, FProfile.addCalledTargetSamples(
409-
LineOffset, Discriminator,
410-
FunctionId(name_count.first),
411-
name_count.second));
408+
mergeSampleProfErrors(Result, FProfile.addCalledTargetSamples(
409+
LineOffset, Discriminator,
410+
FunctionId(name_count.first),
411+
name_count.second));
412412
}
413-
MergeResult(Result, FProfile.addBodySamples(LineOffset, Discriminator,
414-
NumSamples));
413+
mergeSampleProfErrors(
414+
Result,
415+
FProfile.addBodySamples(LineOffset, Discriminator, NumSamples));
415416
break;
416417
}
417418
case LineType::Metadata: {
@@ -892,12 +893,12 @@ std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
892893
if ((useMD5() && FuncGuidsToUse.count(FName.getHashCode())) ||
893894
(!useMD5() && (FuncsToUse.count(FNameString) ||
894895
(Remapper && Remapper->exist(FNameString))))) {
895-
if (!CommonContext || !CommonContext->IsPrefixOf(FContext))
896+
if (!CommonContext || !CommonContext->isPrefixOf(FContext))
896897
CommonContext = &FContext;
897898
}
898899

899900
if (CommonContext == &FContext ||
900-
(CommonContext && CommonContext->IsPrefixOf(FContext))) {
901+
(CommonContext && CommonContext->isPrefixOf(FContext))) {
901902
// Load profile for the current context which originated from
902903
// the common ancestor.
903904
const uint8_t *FuncProfileAddr = Start + NameOffset.second;

llvm/lib/Transforms/IPO/SampleContextTracker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ void SampleContextTracker::createContextLessProfileMap(
624624
FunctionSamples *FProfile = Node->getFunctionSamples();
625625
// Profile's context can be empty, use ContextNode's func name.
626626
if (FProfile)
627-
ContextLessProfiles.Create(Node->getFuncName()).merge(*FProfile);
627+
ContextLessProfiles.create(Node->getFuncName()).merge(*FProfile);
628628
}
629629
}
630630
} // namespace llvm

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,7 @@ void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
15721572
FunctionId(FunctionSamples::getCanonicalFnName(Callee->getName()))];
15731573
OutlineFS->merge(*FS, 1);
15741574
// Set outlined profile to be synthetic to not bias the inliner.
1575-
OutlineFS->SetContextSynthetic();
1575+
OutlineFS->setContextSynthetic();
15761576
}
15771577
} else {
15781578
auto pair =
@@ -1586,7 +1586,7 @@ void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
15861586
static SmallVector<InstrProfValueData, 2>
15871587
GetSortedValueDataFromCallTargets(const SampleRecord::CallTargetMap &M) {
15881588
SmallVector<InstrProfValueData, 2> R;
1589-
for (const auto &I : SampleRecord::SortCallTargets(M)) {
1589+
for (const auto &I : SampleRecord::sortCallTargets(M)) {
15901590
R.emplace_back(
15911591
InstrProfValueData{I.first.getHashCode(), I.second});
15921592
}

llvm/tools/llvm-profdata/llvm-profdata.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,8 @@ remapSamples(const sampleprof::FunctionSamples &Samples,
14211421
for (const auto &Callsite : CallsiteSamples.second) {
14221422
sampleprof::FunctionSamples Remapped =
14231423
remapSamples(Callsite.second, Remapper, Error);
1424-
MergeResult(Error, Target[Remapped.getFunction()].merge(Remapped));
1424+
mergeSampleProfErrors(Error,
1425+
Target[Remapped.getFunction()].merge(Remapped));
14251426
}
14261427
}
14271428
return Result;
@@ -1542,7 +1543,8 @@ static void mergeSampleProfile(const WeightedFileVector &Inputs,
15421543
: FunctionSamples();
15431544
FunctionSamples &Samples = Remapper ? Remapped : I->second;
15441545
SampleContext FContext = Samples.getContext();
1545-
MergeResult(Result, ProfileMap[FContext].merge(Samples, Input.Weight));
1546+
mergeSampleProfErrors(Result,
1547+
ProfileMap[FContext].merge(Samples, Input.Weight));
15461548
if (Result != sampleprof_error::success) {
15471549
std::error_code EC = make_error_code(Result);
15481550
handleMergeWriterError(errorCodeToError(EC), Input.Filename,

llvm/tools/llvm-profgen/ProfileGenerator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ bool CSProfileGenerator::collectFunctionsFromLLVMProfile(
489489
FunctionSamples &
490490
ProfileGenerator::getTopLevelFunctionProfile(FunctionId FuncName) {
491491
SampleContext Context(FuncName);
492-
return ProfileMap.Create(Context);
492+
return ProfileMap.create(Context);
493493
}
494494

495495
void ProfileGenerator::generateProfile() {

0 commit comments

Comments
 (0)