https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/215426
>From f85d866e7bdf73a42f6fdacca9ee656a8e9b1dab 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 4d7b92b7179db..c74705acc98a5 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -1750,7 +1750,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) @@ -1792,21 +1792,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
