[NFC] Coding style fixes: SampleProf

[mtrofin]

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

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Mircea Trofin (mtrofin)

Changes

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 --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,