diff options
author | Mircea Trofin <mtrofin@google.com> | 2024-07-09 14:35:49 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-09 14:35:49 -0700 |
commit | ce03155a1bd02fc078f710b5497be4e382bbe5d9 (patch) | |
tree | b19c81a46fd1cbb69ae26b8060c0793e1c67dba8 | |
parent | f363e30f15ef274f94dba53a13d73998066a0148 (diff) | |
download | llvm-ce03155a1bd02fc078f710b5497be4e382bbe5d9.zip llvm-ce03155a1bd02fc078f710b5497be4e382bbe5d9.tar.gz llvm-ce03155a1bd02fc078f710b5497be4e382bbe5d9.tar.bz2 |
[NFC] Coding style fixes: SampleProf (#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
-rw-r--r-- | llvm/include/llvm/ProfileData/SampleProf.h | 92 | ||||
-rw-r--r-- | llvm/lib/ProfileData/SampleProf.cpp | 4 | ||||
-rw-r--r-- | llvm/lib/ProfileData/SampleProfReader.cpp | 25 | ||||
-rw-r--r-- | llvm/lib/Transforms/IPO/SampleContextTracker.cpp | 2 | ||||
-rw-r--r-- | llvm/lib/Transforms/IPO/SampleProfile.cpp | 4 | ||||
-rw-r--r-- | llvm/tools/llvm-profdata/llvm-profdata.cpp | 6 | ||||
-rw-r--r-- | llvm/tools/llvm-profgen/ProfileGenerator.cpp | 2 |
7 files changed, 70 insertions, 65 deletions
diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h index 51d590b..5c2a78c 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 @@ public: 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 @@ public: } /// 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 @@ public: } /// 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; } @@ -692,7 +693,7 @@ public: } }; - 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 @@ public: } // 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 @@ public: 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 @@ public: /// 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 @@ public: /// 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 @@ public: /// 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 @@ public: 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 @@ public: 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 @@ public: 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 @@ public: 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 @@ public: } /// 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 @@ public: /// 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 @@ public: // 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 @@ public: 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 @@ public: 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 59fa718..294f646 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 a4b2d06..4752465 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/SampleContextTracker.cpp b/llvm/lib/Transforms/IPO/SampleContextTracker.cpp index f7a54d4..f878e3e 100644 --- a/llvm/lib/Transforms/IPO/SampleContextTracker.cpp +++ b/llvm/lib/Transforms/IPO/SampleContextTracker.cpp @@ -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 diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp index d11b0b7..13c0e0d 100644 --- a/llvm/lib/Transforms/IPO/SampleProfile.cpp +++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp @@ -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 = @@ -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}); } diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp index 2ce0668..39cc87c 100644 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -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; @@ -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, diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp index 2118e95..53a25b2 100644 --- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp +++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp @@ -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() { |