From 9f1fb0718fc400458d639dc26e204d35a8a3b43e Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 17 Sep 2025 11:34:59 +0100 Subject: [PATCH 1/4] [BOLT][AArch64] Refuse to run CDSplit pass LongJmp does not support warm blocks. On builds without assertions, this may lead to unexpected crashes. This patch exits with a clear message. --- bolt/include/bolt/Passes/SplitFunctions.h | 19 ------------ bolt/include/bolt/Utils/CommandLineOpts.h | 20 +++++++++++++ bolt/lib/Passes/LongJmp.cpp | 4 +++ bolt/lib/Passes/SplitFunctions.cpp | 36 +++++------------------ bolt/lib/Utils/CommandLineOpts.cpp | 23 +++++++++++++++ bolt/test/AArch64/unsupported-passes.test | 8 +++-- 6 files changed, 60 insertions(+), 50 deletions(-) diff --git a/bolt/include/bolt/Passes/SplitFunctions.h b/bolt/include/bolt/Passes/SplitFunctions.h index 8bdc48b68eb7a..2c1bf1890cd97 100644 --- a/bolt/include/bolt/Passes/SplitFunctions.h +++ b/bolt/include/bolt/Passes/SplitFunctions.h @@ -18,25 +18,6 @@ namespace llvm { namespace bolt { -/// Strategy used to partition blocks into fragments. -enum SplitFunctionsStrategy : char { - /// Split each function into a hot and cold fragment using profiling - /// information. - Profile2 = 0, - /// Split each function into a hot, warm, and cold fragment using - /// profiling information. - CDSplit, - /// Split each function into a hot and cold fragment at a randomly chosen - /// split point (ignoring any available profiling information). - Random2, - /// Split each function into N fragments at a randomly chosen split points - /// (ignoring any available profiling information). - RandomN, - /// Split all basic blocks of each function into fragments such that each - /// fragment contains exactly a single basic block. - All -}; - class SplitStrategy { public: using BlockIt = BinaryFunction::BasicBlockOrderType::iterator; diff --git a/bolt/include/bolt/Utils/CommandLineOpts.h b/bolt/include/bolt/Utils/CommandLineOpts.h index 859d6f3bf6774..0964c2c9d8473 100644 --- a/bolt/include/bolt/Utils/CommandLineOpts.h +++ b/bolt/include/bolt/Utils/CommandLineOpts.h @@ -29,6 +29,25 @@ enum HeatmapModeKind { HM_Optional // perf2bolt --heatmap }; +/// Strategy used to partition blocks into fragments. +enum SplitFunctionsStrategy : char { + /// Split each function into a hot and cold fragment using profiling + /// information. + Profile2 = 0, + /// Split each function into a hot, warm, and cold fragment using + /// profiling information. + CDSplit, + /// Split each function into a hot and cold fragment at a randomly chosen + /// split point (ignoring any available profiling information). + Random2, + /// Split each function into N fragments at a randomly chosen split points + /// (ignoring any available profiling information). + RandomN, + /// Split all basic blocks of each function into fragments such that each + /// fragment contains exactly a single basic block. + All +}; + using HeatmapBlockSizes = std::vector; struct HeatmapBlockSpecParser : public llvm::cl::parser { explicit HeatmapBlockSpecParser(llvm::cl::Option &O) @@ -78,6 +97,7 @@ extern llvm::cl::opt OutputFilename; extern llvm::cl::opt PerfData; extern llvm::cl::opt PrintCacheMetrics; extern llvm::cl::opt PrintSections; +extern llvm::cl::opt SplitStrategy; // The format to use with -o in aggregation mode (perf2bolt) enum ProfileFormatKind { PF_Fdata, PF_YAML }; diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index 4dade161cc232..e62cbe4e6d9d5 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -894,6 +894,10 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF) { } Error LongJmpPass::runOnFunctions(BinaryContext &BC) { + if (opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) { + BC.errs() << "BOLT-ERROR: CDSplit is not supported on AArch64\n"; + exit(1); + } if (opts::CompactCodeModel) { BC.outs() diff --git a/bolt/lib/Passes/SplitFunctions.cpp b/bolt/lib/Passes/SplitFunctions.cpp index b21401e069bfa..37243c5f3c7a1 100644 --- a/bolt/lib/Passes/SplitFunctions.cpp +++ b/bolt/lib/Passes/SplitFunctions.cpp @@ -85,29 +85,7 @@ static cl::opt SplitThreshold( "size is reduced. Note that on some architectures the size can " "increase after splitting."), cl::init(0), cl::Hidden, cl::cat(BoltOptCategory)); - -static cl::opt SplitStrategy( - "split-strategy", cl::init(SplitFunctionsStrategy::Profile2), - cl::values(clEnumValN(SplitFunctionsStrategy::Profile2, "profile2", - "split each function into a hot and cold fragment " - "using profiling information")), - cl::values(clEnumValN(SplitFunctionsStrategy::CDSplit, "cdsplit", - "split each function into a hot, warm, and cold " - "fragment using profiling information")), - cl::values(clEnumValN( - SplitFunctionsStrategy::Random2, "random2", - "split each function into a hot and cold fragment at a randomly chosen " - "split point (ignoring any available profiling information)")), - cl::values(clEnumValN( - SplitFunctionsStrategy::RandomN, "randomN", - "split each function into N fragments at a randomly chosen split " - "points (ignoring any available profiling information)")), - cl::values(clEnumValN( - SplitFunctionsStrategy::All, "all", - "split all basic blocks of each function into fragments such that each " - "fragment contains exactly a single basic block")), - cl::desc("strategy used to partition blocks into fragments"), - cl::cat(BoltOptCategory)); + static cl::opt CallScale( "call-scale", @@ -724,14 +702,14 @@ Error SplitFunctions::runOnFunctions(BinaryContext &BC) { // If split strategy is not CDSplit, then a second run of the pass is not // needed after function reordering. if (BC.HasFinalizedFunctionOrder && - opts::SplitStrategy != SplitFunctionsStrategy::CDSplit) + opts::SplitStrategy != opts::SplitFunctionsStrategy::CDSplit) return Error::success(); std::unique_ptr Strategy; bool ForceSequential = false; switch (opts::SplitStrategy) { - case SplitFunctionsStrategy::CDSplit: + case opts::SplitFunctionsStrategy::CDSplit: // CDSplit runs two splitting passes: hot-cold splitting (SplitPrfoile2) // before function reordering and hot-warm-cold splitting // (SplitCacheDirected) after function reordering. @@ -742,21 +720,21 @@ Error SplitFunctions::runOnFunctions(BinaryContext &BC) { opts::AggressiveSplitting = true; BC.HasWarmSection = true; break; - case SplitFunctionsStrategy::Profile2: + case opts::SplitFunctionsStrategy::Profile2: Strategy = std::make_unique(); break; - case SplitFunctionsStrategy::Random2: + case opts::SplitFunctionsStrategy::Random2: Strategy = std::make_unique(); // If we split functions randomly, we need to ensure that across runs with // the same input, we generate random numbers for each function in the same // order. ForceSequential = true; break; - case SplitFunctionsStrategy::RandomN: + case opts::SplitFunctionsStrategy::RandomN: Strategy = std::make_unique(); ForceSequential = true; break; - case SplitFunctionsStrategy::All: + case opts::SplitFunctionsStrategy::All: Strategy = std::make_unique(); break; } diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp index 5635da476451d..095612ac3a4ac 100644 --- a/bolt/lib/Utils/CommandLineOpts.cpp +++ b/bolt/lib/Utils/CommandLineOpts.cpp @@ -104,6 +104,29 @@ ExecutionCountThreshold("execution-count-threshold", cl::Hidden, cl::cat(BoltOptCategory)); +cl::opt SplitStrategy( + "split-strategy", cl::init(SplitFunctionsStrategy::Profile2), + cl::values(clEnumValN(SplitFunctionsStrategy::Profile2, "profile2", + "split each function into a hot and cold fragment " + "using profiling information")), + cl::values(clEnumValN(SplitFunctionsStrategy::CDSplit, "cdsplit", + "split each function into a hot, warm, and cold " + "fragment using profiling information")), + cl::values(clEnumValN( + SplitFunctionsStrategy::Random2, "random2", + "split each function into a hot and cold fragment at a randomly chosen " + "split point (ignoring any available profiling information)")), + cl::values(clEnumValN( + SplitFunctionsStrategy::RandomN, "randomN", + "split each function into N fragments at a randomly chosen split " + "points (ignoring any available profiling information)")), + cl::values(clEnumValN( + SplitFunctionsStrategy::All, "all", + "split all basic blocks of each function into fragments such that each " + "fragment contains exactly a single basic block")), + cl::desc("strategy used to partition blocks into fragments"), + cl::cat(BoltOptCategory)); + bool HeatmapBlockSpecParser::parse(cl::Option &O, StringRef ArgName, StringRef Arg, HeatmapBlockSizes &Val) { // Parses a human-readable suffix into a shift amount or nullopt on error. diff --git a/bolt/test/AArch64/unsupported-passes.test b/bolt/test/AArch64/unsupported-passes.test index 886fc1c574dcf..f70c4d42e2929 100644 --- a/bolt/test/AArch64/unsupported-passes.test +++ b/bolt/test/AArch64/unsupported-passes.test @@ -3,6 +3,10 @@ // REQUIRES: system-linux,asserts,target=aarch64{{.*}} RUN: %clang %cflags %p/../Inputs/hello.c -o %t -Wl,-q -RUN: not llvm-bolt %t -o %t.bolt --frame-opt=all 2>&1 | FileCheck %s +RUN: not llvm-bolt %t -o %t.bolt --frame-opt=all 2>&1 | FileCheck %s --check-prefix=CHECK-FRAME-OPT -CHECK: BOLT-ERROR: frame-optimizer is supported only on X86 +CHECK-FRAME-OPT: BOLT-ERROR: frame-optimizer is supported only on X86 + +RUN: not llvm-bolt %t -o %t.bolt split-functions --split-strategy=cdsplit 2>&1 | FileCheck %s --check-prefix=CHECK-CDSPLIT + +CHECK-CDSPLIT: BOLT-ERROR: CDSplit is not supported on AArch64 From 1a1405f432577f67063524f7e58a2c681919ba7f Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 17 Sep 2025 13:55:20 +0100 Subject: [PATCH 2/4] code formatter fix --- bolt/lib/Passes/SplitFunctions.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/bolt/lib/Passes/SplitFunctions.cpp b/bolt/lib/Passes/SplitFunctions.cpp index 37243c5f3c7a1..eab669b32b71e 100644 --- a/bolt/lib/Passes/SplitFunctions.cpp +++ b/bolt/lib/Passes/SplitFunctions.cpp @@ -85,7 +85,6 @@ static cl::opt SplitThreshold( "size is reduced. Note that on some architectures the size can " "increase after splitting."), cl::init(0), cl::Hidden, cl::cat(BoltOptCategory)); - static cl::opt CallScale( "call-scale", From 2d8293101f1f7e940e95abcf01d8256ce2fe661f Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Mon, 29 Sep 2025 13:46:03 +0100 Subject: [PATCH 3/4] Addressing reviewers --- bolt/lib/Passes/LongJmp.cpp | 7 +++++-- bolt/test/AArch64/unsupported-passes.test | 3 +-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index e62cbe4e6d9d5..03fc10709f48d 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -894,8 +894,11 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF) { } Error LongJmpPass::runOnFunctions(BinaryContext &BC) { - if (opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) { - BC.errs() << "BOLT-ERROR: CDSplit is not supported on AArch64\n"; + + if (!opts::CompactCodeModel && + opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) { + BC.errs() << "BOLT-ERROR: CDSplit is not supported with LongJmp. Try with " + "'--compact-code-model'\n"; exit(1); } diff --git a/bolt/test/AArch64/unsupported-passes.test b/bolt/test/AArch64/unsupported-passes.test index f70c4d42e2929..5b12d86500eea 100644 --- a/bolt/test/AArch64/unsupported-passes.test +++ b/bolt/test/AArch64/unsupported-passes.test @@ -8,5 +8,4 @@ RUN: not llvm-bolt %t -o %t.bolt --frame-opt=all 2>&1 | FileCheck %s --check-pre CHECK-FRAME-OPT: BOLT-ERROR: frame-optimizer is supported only on X86 RUN: not llvm-bolt %t -o %t.bolt split-functions --split-strategy=cdsplit 2>&1 | FileCheck %s --check-prefix=CHECK-CDSPLIT - -CHECK-CDSPLIT: BOLT-ERROR: CDSplit is not supported on AArch64 +CHECK-CDSPLIT: BOLT-ERROR: CDSplit is not supported with LongJmp. Try with '--compact-code-model' From cc477f0527475495a27cf3995179f1af6d33bdf5 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 1 Oct 2025 12:22:10 +0100 Subject: [PATCH 4/4] Addressing reviewers (2) --- bolt/lib/Passes/LongJmp.cpp | 9 +++------ bolt/lib/Rewrite/RewriteInstance.cpp | 7 +++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/bolt/lib/Passes/LongJmp.cpp b/bolt/lib/Passes/LongJmp.cpp index 03fc10709f48d..03c1ea9d837e2 100644 --- a/bolt/lib/Passes/LongJmp.cpp +++ b/bolt/lib/Passes/LongJmp.cpp @@ -895,12 +895,9 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF) { Error LongJmpPass::runOnFunctions(BinaryContext &BC) { - if (!opts::CompactCodeModel && - opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) { - BC.errs() << "BOLT-ERROR: CDSplit is not supported with LongJmp. Try with " - "'--compact-code-model'\n"; - exit(1); - } + assert((opts::CompactCodeModel || + opts::SplitStrategy != opts::SplitFunctionsStrategy::CDSplit) && + "LongJmp cannot work with functions split in more than two fragments"); if (opts::CompactCodeModel) { BC.outs() diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index a6e4dbc9c192f..db5628ecd400c 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -2113,6 +2113,13 @@ void RewriteInstance::adjustCommandLineOptions() { opts::SplitEH = false; } + if (BC->isAArch64() && !opts::CompactCodeModel && + opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) { + BC->errs() << "BOLT-ERROR: CDSplit is not supported with LongJmp. Try with " + "'--compact-code-model'\n"; + exit(1); + } + if (opts::StrictMode && !BC->HasRelocations) { BC->errs() << "BOLT-WARNING: disabling strict mode (-strict) in non-relocation "