[BOLT][AArch64] Refuse to run CDSplit pass #159351

paschalis-mpeis · 2025-09-17T12:49:01Z

LongJmp does not support warm blocks.
On builds without assertions, this may lead to unexpected crashes.

This patch exits with a clear message.

LongJmp does not support warm blocks. On builds without assertions, this may lead to unexpected crashes. This patch exits with a clear message.

llvmbot · 2025-09-17T12:49:45Z

@llvm/pr-subscribers-bolt

Author: Paschalis Mpeis (paschalis-mpeis)

Changes

LongJmp does not support warm blocks.
On builds without assertions, this may lead to unexpected crashes.

This patch exits with a clear message.

Full diff: https://github.com/llvm/llvm-project/pull/159351.diff

6 Files Affected:

(modified) bolt/include/bolt/Passes/SplitFunctions.h (-19)
(modified) bolt/include/bolt/Utils/CommandLineOpts.h (+20)
(modified) bolt/lib/Passes/LongJmp.cpp (+4)
(modified) bolt/lib/Passes/SplitFunctions.cpp (+7-29)
(modified) bolt/lib/Utils/CommandLineOpts.cpp (+23)
(modified) bolt/test/AArch64/unsupported-passes.test (+6-2)

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<unsigned>;
 struct HeatmapBlockSpecParser : public llvm::cl::parser<HeatmapBlockSizes> {
   explicit HeatmapBlockSpecParser(llvm::cl::Option &O)
@@ -78,6 +97,7 @@ extern llvm::cl::opt<std::string> OutputFilename;
 extern llvm::cl::opt<std::string> PerfData;
 extern llvm::cl::opt<bool> PrintCacheMetrics;
 extern llvm::cl::opt<bool> PrintSections;
+extern llvm::cl::opt<SplitFunctionsStrategy> 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<unsigned> 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<SplitFunctionsStrategy> 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<double> 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<SplitStrategy> 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<SplitProfile2>();
     break;
-  case SplitFunctionsStrategy::Random2:
+  case opts::SplitFunctionsStrategy::Random2:
     Strategy = std::make_unique<SplitRandom2>();
     // 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<SplitRandomN>();
     ForceSequential = true;
     break;
-  case SplitFunctionsStrategy::All:
+  case opts::SplitFunctionsStrategy::All:
     Strategy = std::make_unique<SplitAll>();
     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<SplitFunctionsStrategy> 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

github-actions · 2025-09-17T12:52:47Z

✅ With the latest revision this PR passed the C/C++ code formatter.

paschalis-mpeis · 2025-09-17T12:58:48Z

Hey folks,

LongJmp does not work with CDSplit. To support it we need to extend the stub groups.
We discussed this some time ago and the consensus was not to modify the pass (#121475).

If preferable, I could split the refactoring, moving SplitStrategy flags to another PR.

There are more passes/functions unsupported, but at least on AArch64 it would prevent some unexpected errors.

maksfb

@paschalis-mpeis, we should allow to run CDSplit under --compact-code-model, right?

paschalis-mpeis · 2025-09-29T13:09:56Z

Yeap, thanks for noticing this. Sent a new patch.
The compact-code-model does not need to check on cold blocks, so it should work with:

--compact-code-model  --split-strategy=cdsplit

maksfb · 2025-09-29T23:21:33Z

Thinking more about it: since it's just the option verification, it makes sense to run it (and report to the user) as early as possible. I'd say either in SplitFunctions or even adjustCommandLineOptions. In LongJmp main pass (not under -compact-code-model, we can add an assertion that the function is split in no more than two fragments.

paschalis-mpeis · 2025-10-01T11:25:38Z

That's even better, thanks for the suggsetion.
I've added an early exit in adjustCommandLineOptions. Please have a look.

maksfb

Thanks!

LongJmp does not support warm blocks. On builds without assertions, this may lead to unexpected crashes. This patch exits with a clear message.

[BOLT][AArch64] Refuse to run CDSplit pass

9f1fb07

LongJmp does not support warm blocks. On builds without assertions, this may lead to unexpected crashes. This patch exits with a clear message.

paschalis-mpeis marked this pull request as ready for review September 17, 2025 12:49

paschalis-mpeis requested review from aaupov, ayermolo, maksfb, rafaelauler, yota9 and yozhu as code owners September 17, 2025 12:49

llvmbot added the BOLT label Sep 17, 2025

code formatter fix

1a1405f

paschalis-mpeis mentioned this pull request Sep 24, 2025

[BOLT][AArch64] Handle OpNegateRAState to enable optimizing binaries with pac-ret hardening #120064

Merged

8 tasks

maksfb reviewed Sep 25, 2025

View reviewed changes

Addressing reviewers

2d82931

Addressing reviewers (2)

cc477f0

maksfb approved these changes Oct 2, 2025

View reviewed changes

paschalis-mpeis merged commit 224a717 into main Oct 3, 2025
9 checks passed

paschalis-mpeis deleted the users/paschalis-mpeis/cdsplit-aarch64-disable branch October 3, 2025 08:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[BOLT][AArch64] Refuse to run CDSplit pass #159351

[BOLT][AArch64] Refuse to run CDSplit pass #159351

Uh oh!

paschalis-mpeis commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

github-actions bot commented Sep 17, 2025 •

edited

Loading

Uh oh!

paschalis-mpeis commented Sep 17, 2025

Uh oh!

maksfb left a comment

Uh oh!

paschalis-mpeis commented Sep 29, 2025

Uh oh!

maksfb commented Sep 29, 2025

Uh oh!

paschalis-mpeis commented Oct 1, 2025

Uh oh!

maksfb left a comment

Uh oh!

Uh oh!

Uh oh!

[BOLT][AArch64] Refuse to run CDSplit pass #159351

[BOLT][AArch64] Refuse to run CDSplit pass #159351

Uh oh!

Conversation

paschalis-mpeis commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

github-actions bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paschalis-mpeis commented Sep 17, 2025

Uh oh!

maksfb left a comment

Choose a reason for hiding this comment

Uh oh!

paschalis-mpeis commented Sep 29, 2025

Uh oh!

maksfb commented Sep 29, 2025

Uh oh!

paschalis-mpeis commented Oct 1, 2025

Uh oh!

maksfb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 17, 2025 •

edited

Loading