https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/215426
>From e9278104cf0e411142db04c0dd85037ebb04a8be Mon Sep 17 00:00:00 2001 From: Amir Ayupov <[email protected]> Date: Sun, 9 Aug 2026 04:19:06 -0700 Subject: [PATCH] [BOLT] Keep the spelling of each heatmap block size The block-size parser turns "64K" into 65536 and discards the original text, keeping it only for error messages. The working set log then has to either reprint the raw value or reformat it back, and reformatting invents a spelling the user did not choose: "1MiB" comes back as "1M". Store the spelling next to the value and echo it. Heatmap file names keep using the numeric value, matching the existing "dumping heatmap with bucket size N" message and the -<size> suffix that tests already expect. Test Plan: updated heatmap-preagg.test --- bolt/include/bolt/Profile/Heatmap.h | 4 ++-- bolt/include/bolt/Utils/CommandLineOpts.h | 8 +++++++- bolt/lib/Profile/DataAggregator.cpp | 10 +++++----- bolt/lib/Profile/Heatmap.cpp | 8 ++++---- bolt/lib/Utils/CommandLineOpts.cpp | 8 ++++++-- bolt/test/X86/heatmap-preagg.test | 2 +- 6 files changed, 25 insertions(+), 15 deletions(-) diff --git a/bolt/include/bolt/Profile/Heatmap.h b/bolt/include/bolt/Profile/Heatmap.h index 268b02c7d093c..6584999315615 100644 --- a/bolt/include/bolt/Profile/Heatmap.h +++ b/bolt/include/bolt/Profile/Heatmap.h @@ -76,9 +76,9 @@ class Heatmap { void print(raw_ostream &OS) const; - void printCDF(StringRef FileName) const; + void printCDF(StringRef FileName, StringRef Label) const; - void printCDF(raw_ostream &OS) const; + void printCDF(raw_ostream &OS, StringRef Label) const; void printSectionHotness(StringRef Filename) const; diff --git a/bolt/include/bolt/Utils/CommandLineOpts.h b/bolt/include/bolt/Utils/CommandLineOpts.h index c162f60d45559..80eeacbfd6ae7 100644 --- a/bolt/include/bolt/Utils/CommandLineOpts.h +++ b/bolt/include/bolt/Utils/CommandLineOpts.h @@ -48,7 +48,13 @@ enum SplitFunctionsStrategy : char { All }; -using HeatmapBlockSizes = std::vector<unsigned>; +/// A bucket size and how it was spelled on the command line, so output can +/// echo "64K" rather than reformatting the value. +struct HeatmapBlockSize { + unsigned Value = 0; + std::string Spec; +}; +using HeatmapBlockSizes = std::vector<HeatmapBlockSize>; struct HeatmapBlockSpecParser : public llvm::cl::parser<HeatmapBlockSizes> { explicit HeatmapBlockSpecParser(llvm::cl::Option &O) : llvm::cl::parser<HeatmapBlockSizes>(O) {} diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index ca50efef4429d..a1eba32e7fbbe 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -1759,7 +1759,7 @@ std::error_code DataAggregator::printLBRHeatMap() { opts::HeatmapMinAddress = KernelBaseAddr; } opts::HeatmapBlockSizes &HMBS = opts::HeatmapBlock; - Heatmap HM(HMBS[0], opts::HeatmapMinAddress, opts::HeatmapMaxAddress, + Heatmap HM(HMBS[0].Value, opts::HeatmapMinAddress, opts::HeatmapMaxAddress, getTextSections(BC)); auto getSymbolValue = [&](const MCSymbol *Symbol) -> uint64_t { if (Symbol) @@ -1801,21 +1801,21 @@ std::error_code DataAggregator::printLBRHeatMap() { HM.print(opts::HeatmapOutput); if (opts::HeatmapOutput == "-") { - HM.printCDF(opts::HeatmapOutput); + HM.printCDF(opts::HeatmapOutput, HMBS.front().Spec); HM.printSectionHotness(opts::HeatmapOutput); } else { - HM.printCDF(opts::HeatmapOutput + ".csv"); + HM.printCDF(opts::HeatmapOutput + ".csv", HMBS.front().Spec); HM.printSectionHotness(opts::HeatmapOutput + "-section-hotness.csv"); } // Provide coarse-grained heatmaps if requested via zoom-out scales - for (const uint64_t NewBucketSize : ArrayRef(HMBS).drop_front()) { + for (const auto &[NewBucketSize, Label] : ArrayRef(HMBS).drop_front()) { HM.resizeBucket(NewBucketSize); if (opts::HeatmapOutput == "-") HM.print(opts::HeatmapOutput); else HM.print(formatv("{0}-{1}", opts::HeatmapOutput, NewBucketSize).str()); // Working set only; the table is emitted once, at the finest granularity. - HM.printCDF(nulls()); + HM.printCDF(nulls(), Label); } return std::error_code(); diff --git a/bolt/lib/Profile/Heatmap.cpp b/bolt/lib/Profile/Heatmap.cpp index 17ec5981df842..ceaa1dd5673e9 100644 --- a/bolt/lib/Profile/Heatmap.cpp +++ b/bolt/lib/Profile/Heatmap.cpp @@ -251,17 +251,17 @@ void Heatmap::print(raw_ostream &OS) const { } } -void Heatmap::printCDF(StringRef FileName) const { +void Heatmap::printCDF(StringRef FileName, StringRef Label) const { std::error_code EC; raw_fd_ostream OS(FileName, EC, sys::fs::OpenFlags::OF_None); if (EC) { errs() << "error opening output file: " << EC.message() << '\n'; exit(1); } - printCDF(OS); + printCDF(OS, Label); } -void Heatmap::printCDF(raw_ostream &OS) const { +void Heatmap::printCDF(raw_ostream &OS, StringRef Label) const { uint64_t NumTotalCounts = 0; std::vector<uint64_t> Counts; @@ -294,7 +294,7 @@ void Heatmap::printCDF(raw_ostream &OS) const { << format("%.4f", RatioRightInPercent * (RunningCount)) << "\n"; } - outs() << "HEATMAP: working set @ bucket size " << BucketSize << " p" + outs() << "HEATMAP: working set @ bucket size " << Label << " p" << format("%g", CutOff / 10000.0) << "/total: " << NumBuckets << "/" << Counts.size() << '\n'; } diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp index 4d7d8a408070b..ddc5af47228ff 100644 --- a/bolt/lib/Utils/CommandLineOpts.cpp +++ b/bolt/lib/Utils/CommandLineOpts.cpp @@ -192,7 +192,9 @@ bool HeatmapBlockSpecParser::parse(cl::Option &O, StringRef ArgName, unsigned PreviousSize = 0; for (StringRef Size : Sizes) { StringRef OrigSize = Size; - unsigned &SizeVal = Val.emplace_back(0); + HeatmapBlockSize &Block = Val.emplace_back(); + Block.Spec = OrigSize.str(); + unsigned &SizeVal = Block.Value; if (Size.consumeInteger(10, SizeVal)) { O.error("'" + OrigSize + "' value can't be parsed as an integer"); return true; @@ -217,7 +219,9 @@ cl::opt<opts::HeatmapBlockSizes, false, opts::HeatmapBlockSpecParser> "block-size", cl::value_desc("initial_size{,zoom-out_size,...}"), cl::desc("heatmap bucket size, optionally followed by zoom-out sizes " "for coarse-grained heatmaps (default 64B, 4K, 256K)."), - cl::init(HeatmapBlockSizes{/*Initial*/ 64, /*Zoom-out*/ 4096, 262144}), + cl::init(HeatmapBlockSizes{/*Initial*/ {64, "64"}, + /*Zoom-out*/ {4096, "4K"}, + {262144, "256K"}}), cl::cat(HeatmapCategory)); cl::opt<int> HeatmapCdfPct( diff --git a/bolt/test/X86/heatmap-preagg.test b/bolt/test/X86/heatmap-preagg.test index 7ff15047d3f70..e5e0419f2b6de 100644 --- a/bolt/test/X86/heatmap-preagg.test +++ b/bolt/test/X86/heatmap-preagg.test @@ -44,7 +44,7 @@ CHECK-HEATMAP: HEATMAP: working set @ bucket size 64 p99/total: 28/43 CHECK-HEATMAP: HEATMAP: dumping heatmap with bucket size 128 CHECK-HEATMAP: HEATMAP: working set @ bucket size 128 p99/total: 16/23 CHECK-HEATMAP: HEATMAP: dumping heatmap with bucket size 1024 -CHECK-HEATMAP: HEATMAP: working set @ bucket size 1024 p99/total: 3/3 +CHECK-HEATMAP: HEATMAP: working set @ bucket size 1K p99/total: 3/3 CHECK-HEATMAP-NOT: HEATMAP: dumping heatmap with bucket size CHECK-HEATMAP-NOT: HEATMAP: working set @ bucket size _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
