Revert "[NFC][profdata] Apply lints and other format fixes"#149601
Merged
Revert "[NFC][profdata] Apply lints and other format fixes"#149601
Conversation
This reverts commit 4dc6dfd.
Member
|
@llvm/pr-subscribers-pgo Author: Florian Mayer (fmayer) ChangesReverts llvm/llvm-project#149433 This broke the hwasan buildbot: https://lab.llvm.org/buildbot/#/builders/55/builds/14455 Full diff: https://github.com/llvm/llvm-project/pull/149601.diff 1 Files Affected:
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 96d135b9746ff..207ae2ddd4cf2 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -16,6 +16,7 @@
#include "llvm/Debuginfod/HTTPClient.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/Object/Binary.h"
+#include "llvm/ProfileData/DataAccessProf.h"
#include "llvm/ProfileData/InstrProfCorrelator.h"
#include "llvm/ProfileData/InstrProfReader.h"
#include "llvm/ProfileData/InstrProfWriter.h"
@@ -52,23 +53,23 @@ using ProfCorrelatorKind = InstrProfCorrelator::ProfCorrelatorKind;
// https://llvm.org/docs/CommandGuide/llvm-profdata.html has documentations
// on each subcommand.
-static cl::SubCommand ShowSubcommand(
+cl::SubCommand ShowSubcommand(
"show",
"Takes a profile data file and displays the profiles. See detailed "
"documentation in "
"https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-show");
-static cl::SubCommand OrderSubcommand(
+cl::SubCommand OrderSubcommand(
"order",
"Reads temporal profiling traces from a profile and outputs a function "
"order that reduces the number of page faults for those traces. See "
"detailed documentation in "
"https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-order");
-static cl::SubCommand OverlapSubcommand(
+cl::SubCommand OverlapSubcommand(
"overlap",
"Computes and displays the overlap between two profiles. See detailed "
"documentation in "
"https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-overlap");
-static cl::SubCommand MergeSubcommand(
+cl::SubCommand MergeSubcommand(
"merge",
"Takes several profiles and merge them together. See detailed "
"documentation in "
@@ -91,11 +92,12 @@ enum class ShowFormat { Text, Json, Yaml };
} // namespace
// Common options.
-static cl::opt<std::string>
- OutputFilename("output", cl::value_desc("output"), cl::init("-"),
- cl::desc("Output file"), cl::sub(ShowSubcommand),
- cl::sub(OrderSubcommand), cl::sub(OverlapSubcommand),
- cl::sub(MergeSubcommand));
+cl::opt<std::string> OutputFilename("output", cl::value_desc("output"),
+ cl::init("-"), cl::desc("Output file"),
+ cl::sub(ShowSubcommand),
+ cl::sub(OrderSubcommand),
+ cl::sub(OverlapSubcommand),
+ cl::sub(MergeSubcommand));
// NOTE: cl::alias must not have cl::sub(), since aliased option's cl::sub()
// will be used. llvm::cl::alias::done() method asserts this condition.
static cl::alias OutputFilenameA("o", cl::desc("Alias for --output"),
@@ -525,9 +527,9 @@ static void exitWithError(Twine Message, StringRef Whence = "",
static void exitWithError(Error E, StringRef Whence = "") {
if (E.isA<InstrProfError>()) {
handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
- instrprof_error InstrError = IPE.get();
+ instrprof_error instrError = IPE.get();
StringRef Hint = "";
- if (InstrError == instrprof_error::unrecognized_format) {
+ if (instrError == instrprof_error::unrecognized_format) {
// Hint in case user missed specifying the profile type.
Hint = "Perhaps you forgot to use the --sample or --memory option?";
}
@@ -634,7 +636,7 @@ class SymbolRemapper {
return New.empty() ? Name : FunctionId(New);
}
};
-} // namespace
+}
struct WeightedFile {
std::string Filename;
@@ -824,18 +826,18 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
// Only show hint the first time an error occurs.
auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
std::unique_lock<std::mutex> ErrGuard{WC->ErrLock};
- bool FirstTime = WC->WriterErrorCodes.insert(ErrCode).second;
+ bool firstTime = WC->WriterErrorCodes.insert(ErrCode).second;
handleMergeWriterError(make_error<InstrProfError>(ErrCode, Msg),
- Input.Filename, FuncName, FirstTime);
+ Input.Filename, FuncName, firstTime);
});
}
if (KeepVTableSymbols) {
- const InstrProfSymtab &Symtab = Reader->getSymtab();
- const auto &VTableNames = Symtab.getVTableNames();
+ const InstrProfSymtab &symtab = Reader->getSymtab();
+ const auto &VTableNames = symtab.getVTableNames();
- for (const auto &KV : VTableNames)
- WC->Writer.addVTableName(KV.getKey());
+ for (const auto &kv : VTableNames)
+ WC->Writer.addVTableName(kv.getKey());
}
if (Reader->hasTemporalProfile()) {
@@ -876,8 +878,8 @@ static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
auto [ErrorCode, Msg] = InstrProfError::take(std::move(E));
std::unique_lock<std::mutex> ErrGuard{Dst->ErrLock};
- bool FirstTime = Dst->WriterErrorCodes.insert(ErrorCode).second;
- if (FirstTime)
+ bool firstTime = Dst->WriterErrorCodes.insert(ErrorCode).second;
+ if (firstTime)
warn(toString(make_error<InstrProfError>(ErrorCode, Msg)));
});
}
@@ -887,22 +889,24 @@ getFuncName(const StringMap<InstrProfWriter::ProfilingData>::value_type &Val) {
return Val.first();
}
-static std::string getFuncName(const SampleProfileMap::value_type &Val) {
+static std::string
+getFuncName(const SampleProfileMap::value_type &Val) {
return Val.second.getContext().toString();
}
-template <typename T> static void filterFunctions(T &ProfileMap) {
- bool HasFilter = !FuncNameFilter.empty();
- bool HasNegativeFilter = !FuncNameNegativeFilter.empty();
- if (!HasFilter && !HasNegativeFilter)
+template <typename T>
+static void filterFunctions(T &ProfileMap) {
+ bool hasFilter = !FuncNameFilter.empty();
+ bool hasNegativeFilter = !FuncNameNegativeFilter.empty();
+ if (!hasFilter && !hasNegativeFilter)
return;
// If filter starts with '?' it is MSVC mangled name, not a regex.
llvm::Regex ProbablyMSVCMangledName("[?@$_0-9A-Za-z]+");
- if (HasFilter && FuncNameFilter[0] == '?' &&
+ if (hasFilter && FuncNameFilter[0] == '?' &&
ProbablyMSVCMangledName.match(FuncNameFilter))
FuncNameFilter = llvm::Regex::escape(FuncNameFilter);
- if (HasNegativeFilter && FuncNameNegativeFilter[0] == '?' &&
+ if (hasNegativeFilter && FuncNameNegativeFilter[0] == '?' &&
ProbablyMSVCMangledName.match(FuncNameNegativeFilter))
FuncNameNegativeFilter = llvm::Regex::escape(FuncNameNegativeFilter);
@@ -910,9 +914,9 @@ template <typename T> static void filterFunctions(T &ProfileMap) {
llvm::Regex Pattern(FuncNameFilter);
llvm::Regex NegativePattern(FuncNameNegativeFilter);
std::string Error;
- if (HasFilter && !Pattern.isValid(Error))
+ if (hasFilter && !Pattern.isValid(Error))
exitWithError(Error);
- if (HasNegativeFilter && !NegativePattern.isValid(Error))
+ if (hasNegativeFilter && !NegativePattern.isValid(Error))
exitWithError(Error);
// Handle MD5 profile, so it is still able to match using the original name.
@@ -924,10 +928,10 @@ template <typename T> static void filterFunctions(T &ProfileMap) {
auto Tmp = I++;
const auto &FuncName = getFuncName(*Tmp);
// Negative filter has higher precedence than positive filter.
- if ((HasNegativeFilter &&
+ if ((hasNegativeFilter &&
(NegativePattern.match(FuncName) ||
(FunctionSamples::UseMD5 && NegativeMD5Name == FuncName))) ||
- (HasFilter && !(Pattern.match(FuncName) ||
+ (hasFilter && !(Pattern.match(FuncName) ||
(FunctionSamples::UseMD5 && MD5Name == FuncName))))
ProfileMap.erase(Tmp);
}
@@ -1188,7 +1192,7 @@ adjustInstrProfile(std::unique_ptr<WriterContext> &WC,
StringMap<StringRef> StaticFuncMap;
InstrProfSummaryBuilder IPBuilder(ProfileSummaryBuilder::DefaultCutoffs);
- auto CheckSampleProfileHasFUnique = [&Reader]() {
+ auto checkSampleProfileHasFUnique = [&Reader]() {
for (const auto &PD : Reader->getProfiles()) {
auto &FContext = PD.second.getContext();
if (FContext.toString().find(FunctionSamples::UniqSuffix) !=
@@ -1199,9 +1203,9 @@ adjustInstrProfile(std::unique_ptr<WriterContext> &WC,
return false;
};
- bool SampleProfileHasFUnique = CheckSampleProfileHasFUnique();
+ bool SampleProfileHasFUnique = checkSampleProfileHasFUnique();
- auto BuildStaticFuncMap = [&StaticFuncMap,
+ auto buildStaticFuncMap = [&StaticFuncMap,
SampleProfileHasFUnique](const StringRef Name) {
std::string FilePrefixes[] = {".cpp", "cc", ".c", ".hpp", ".h"};
size_t PrefixPos = StringRef::npos;
@@ -1361,7 +1365,7 @@ adjustInstrProfile(std::unique_ptr<WriterContext> &WC,
InstrProfRecord *R = &PD.getValue().begin()->second;
StringRef FullName = PD.getKey();
InstrProfileMap[FullName] = InstrProfileEntry(R);
- BuildStaticFuncMap(FullName);
+ buildStaticFuncMap(FullName);
}
for (auto &PD : Reader->getProfiles()) {
@@ -1492,8 +1496,8 @@ remapSamples(const sampleprof::FunctionSamples &Samples,
BodySample.second.getSamples());
for (const auto &Target : BodySample.second.getCallTargets()) {
Result.addCalledTargetSamples(BodySample.first.LineOffset,
- MaskedDiscriminator, Remapper(Target.first),
- Target.second);
+ MaskedDiscriminator,
+ Remapper(Target.first), Target.second);
}
}
for (const auto &CallsiteSamples : Samples.getCallsiteSamples()) {
@@ -1754,7 +1758,7 @@ static void parseInputFilenamesFile(MemoryBuffer *Buffer,
if (SanitizedEntry.starts_with("#"))
continue;
// If there's no comma, it's an unweighted profile.
- if (!SanitizedEntry.contains(','))
+ else if (!SanitizedEntry.contains(','))
addWeightedInput(WFV, {std::string(SanitizedEntry), 1});
else
addWeightedInput(WFV, parseWeightedFile(SanitizedEntry));
@@ -2735,11 +2739,10 @@ std::error_code SampleOverlapAggregator::loadProfiles() {
return std::error_code();
}
-static void overlapSampleProfile(const std::string &BaseFilename,
- const std::string &TestFilename,
- const OverlapFuncFilters &FuncFilter,
- uint64_t SimilarityCutoff,
- raw_fd_ostream &OS) {
+void overlapSampleProfile(const std::string &BaseFilename,
+ const std::string &TestFilename,
+ const OverlapFuncFilters &FuncFilter,
+ uint64_t SimilarityCutoff, raw_fd_ostream &OS) {
using namespace sampleprof;
// We use 0.000005 to initialize OverlapAggr.Epsilon because the final metrics
@@ -2870,7 +2873,7 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
OS << ":ir\n";
for (const auto &Func : *Reader) {
- if (IsIRInstr) {
+ if (Reader->isIRLevelProfile()) {
bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
if (FuncIsCS != ShowCS)
continue;
@@ -2878,7 +2881,9 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
bool Show = ShowAllFunctions ||
(!FuncNameFilter.empty() && Func.Name.contains(FuncNameFilter));
- if (Show && TextFormat) {
+ bool doTextFormatDump = (Show && TextFormat);
+
+ if (doTextFormatDump) {
InstrProfSymtab &Symtab = Reader->getSymtab();
InstrProfWriter::writeRecordInText(Func.Name, Func.Hash, Func, Symtab,
OS);
@@ -2916,9 +2921,9 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
continue;
}
- for (const auto &Count : Func.Counts) {
- FuncMax = std::max(FuncMax, Count);
- FuncSum += Count;
+ for (size_t I = 0, E = Func.Counts.size(); I < E; ++I) {
+ FuncMax = std::max(FuncMax, Func.Counts[I]);
+ FuncSum += Func.Counts[I];
}
if (FuncMax < ShowValueCutoff) {
@@ -2928,8 +2933,7 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
<< " Sum = " << FuncSum << ")\n";
}
continue;
- }
- if (OnlyListBelow)
+ } else if (OnlyListBelow)
continue;
if (TopNFunctions || ShowHotFuncList)
@@ -2996,8 +3000,9 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
if (TextFormat || ShowCovered)
return 0;
std::unique_ptr<ProfileSummary> PS(Builder.getSummary());
- OS << "Instrumentation level: " << (IsIRInstr ? "IR" : "Front-end");
- if (IsIRInstr) {
+ bool IsIR = Reader->isIRLevelProfile();
+ OS << "Instrumentation level: " << (IsIR ? "IR" : "Front-end");
+ if (IsIR) {
OS << " entry_first = " << Reader->instrEntryBBEnabled();
OS << " instrument_loop_entries = " << Reader->instrLoopEntriesEnabled();
}
@@ -3065,10 +3070,10 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
auto &Traces = Reader->getTemporalProfTraces();
OS << "Temporal Profile Traces (samples=" << Traces.size()
<< " seen=" << Reader->getTemporalProfTraceStreamSize() << "):\n";
- for (auto [Index, Trace] : llvm::enumerate(Traces)) {
- OS << " Temporal Profile Trace " << Index << " (weight=" << Trace.Weight
- << " count=" << Trace.FunctionNameRefs.size() << "):\n";
- for (auto &NameRef : Trace.FunctionNameRefs)
+ for (unsigned i = 0; i < Traces.size(); i++) {
+ OS << " Temporal Profile Trace " << i << " (weight=" << Traces[i].Weight
+ << " count=" << Traces[i].FunctionNameRefs.size() << "):\n";
+ for (auto &NameRef : Traces[i].FunctionNameRefs)
OS << " " << Reader->getSymtab().getFuncOrVarName(NameRef) << "\n";
}
}
@@ -3381,8 +3386,7 @@ static int show_main(StringRef ProgName) {
exitWithErrorCode(EC, OutputFilename);
if (ShowAllFunctions && !FuncNameFilter.empty())
- WithColor::warning()
- << "-function argument ignored: showing all functions\n";
+ WithColor::warning() << "-function argument ignored: showing all functions\n";
if (!DebugInfoFilename.empty())
return showDebugInfoCorrelation(DebugInfoFilename, SFormat, OS);
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/tools/llvm-profdata/llvm-profdata.cppView the diff from clang-format here.diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 207ae2ddd..d6c79cf43 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -636,7 +636,7 @@ public:
return New.empty() ? Name : FunctionId(New);
}
};
-}
+} // namespace
struct WeightedFile {
std::string Filename;
@@ -889,13 +889,11 @@ getFuncName(const StringMap<InstrProfWriter::ProfilingData>::value_type &Val) {
return Val.first();
}
-static std::string
-getFuncName(const SampleProfileMap::value_type &Val) {
+static std::string getFuncName(const SampleProfileMap::value_type &Val) {
return Val.second.getContext().toString();
}
-template <typename T>
-static void filterFunctions(T &ProfileMap) {
+template <typename T> static void filterFunctions(T &ProfileMap) {
bool hasFilter = !FuncNameFilter.empty();
bool hasNegativeFilter = !FuncNameNegativeFilter.empty();
if (!hasFilter && !hasNegativeFilter)
@@ -1496,8 +1494,8 @@ remapSamples(const sampleprof::FunctionSamples &Samples,
BodySample.second.getSamples());
for (const auto &Target : BodySample.second.getCallTargets()) {
Result.addCalledTargetSamples(BodySample.first.LineOffset,
- MaskedDiscriminator,
- Remapper(Target.first), Target.second);
+ MaskedDiscriminator, Remapper(Target.first),
+ Target.second);
}
}
for (const auto &CallsiteSamples : Samples.getCallsiteSamples()) {
@@ -3386,7 +3384,8 @@ static int show_main(StringRef ProgName) {
exitWithErrorCode(EC, OutputFilename);
if (ShowAllFunctions && !FuncNameFilter.empty())
- WithColor::warning() << "-function argument ignored: showing all functions\n";
+ WithColor::warning()
+ << "-function argument ignored: showing all functions\n";
if (!DebugInfoFilename.empty())
return showDebugInfoCorrelation(DebugInfoFilename, SFormat, OS);
|
ellishg
approved these changes
Jul 18, 2025
Contributor
ellishg
left a comment
ellishg
left a comment
There was a problem hiding this comment.
Thanks, I'm looking into it now and I'm very confused how it was affected by this patch
This was referenced Jul 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #149433
This broke the hwasan buildbot: https://lab.llvm.org/buildbot/#/builders/55/builds/14455