Skip to content

Commit 224a717

Browse files
[BOLT][AArch64] Refuse to run CDSplit pass (#159351)
LongJmp does not support warm blocks. On builds without assertions, this may lead to unexpected crashes. This patch exits with a clear message.
1 parent 19cd5bd commit 224a717

File tree

7 files changed

+65
-50
lines changed

7 files changed

+65
-50
lines changed

bolt/include/bolt/Passes/SplitFunctions.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,6 @@
1818
namespace llvm {
1919
namespace bolt {
2020

21-
/// Strategy used to partition blocks into fragments.
22-
enum SplitFunctionsStrategy : char {
23-
/// Split each function into a hot and cold fragment using profiling
24-
/// information.
25-
Profile2 = 0,
26-
/// Split each function into a hot, warm, and cold fragment using
27-
/// profiling information.
28-
CDSplit,
29-
/// Split each function into a hot and cold fragment at a randomly chosen
30-
/// split point (ignoring any available profiling information).
31-
Random2,
32-
/// Split each function into N fragments at a randomly chosen split points
33-
/// (ignoring any available profiling information).
34-
RandomN,
35-
/// Split all basic blocks of each function into fragments such that each
36-
/// fragment contains exactly a single basic block.
37-
All
38-
};
39-
4021
class SplitStrategy {
4122
public:
4223
using BlockIt = BinaryFunction::BasicBlockOrderType::iterator;

bolt/include/bolt/Utils/CommandLineOpts.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ enum HeatmapModeKind {
2929
HM_Optional // perf2bolt --heatmap
3030
};
3131

32+
/// Strategy used to partition blocks into fragments.
33+
enum SplitFunctionsStrategy : char {
34+
/// Split each function into a hot and cold fragment using profiling
35+
/// information.
36+
Profile2 = 0,
37+
/// Split each function into a hot, warm, and cold fragment using
38+
/// profiling information.
39+
CDSplit,
40+
/// Split each function into a hot and cold fragment at a randomly chosen
41+
/// split point (ignoring any available profiling information).
42+
Random2,
43+
/// Split each function into N fragments at a randomly chosen split points
44+
/// (ignoring any available profiling information).
45+
RandomN,
46+
/// Split all basic blocks of each function into fragments such that each
47+
/// fragment contains exactly a single basic block.
48+
All
49+
};
50+
3251
using HeatmapBlockSizes = std::vector<unsigned>;
3352
struct HeatmapBlockSpecParser : public llvm::cl::parser<HeatmapBlockSizes> {
3453
explicit HeatmapBlockSpecParser(llvm::cl::Option &O)
@@ -78,6 +97,7 @@ extern llvm::cl::opt<std::string> OutputFilename;
7897
extern llvm::cl::opt<std::string> PerfData;
7998
extern llvm::cl::opt<bool> PrintCacheMetrics;
8099
extern llvm::cl::opt<bool> PrintSections;
100+
extern llvm::cl::opt<SplitFunctionsStrategy> SplitStrategy;
81101

82102
// The format to use with -o in aggregation mode (perf2bolt)
83103
enum ProfileFormatKind { PF_Fdata, PF_YAML };

bolt/lib/Passes/LongJmp.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,10 @@ void LongJmpPass::relaxLocalBranches(BinaryFunction &BF) {
895895

896896
Error LongJmpPass::runOnFunctions(BinaryContext &BC) {
897897

898+
assert((opts::CompactCodeModel ||
899+
opts::SplitStrategy != opts::SplitFunctionsStrategy::CDSplit) &&
900+
"LongJmp cannot work with functions split in more than two fragments");
901+
898902
if (opts::CompactCodeModel) {
899903
BC.outs()
900904
<< "BOLT-INFO: relaxing branches for compact code model (<128MB)\n";

bolt/lib/Passes/SplitFunctions.cpp

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -86,29 +86,6 @@ static cl::opt<unsigned> SplitThreshold(
8686
"increase after splitting."),
8787
cl::init(0), cl::Hidden, cl::cat(BoltOptCategory));
8888

89-
static cl::opt<SplitFunctionsStrategy> SplitStrategy(
90-
"split-strategy", cl::init(SplitFunctionsStrategy::Profile2),
91-
cl::values(clEnumValN(SplitFunctionsStrategy::Profile2, "profile2",
92-
"split each function into a hot and cold fragment "
93-
"using profiling information")),
94-
cl::values(clEnumValN(SplitFunctionsStrategy::CDSplit, "cdsplit",
95-
"split each function into a hot, warm, and cold "
96-
"fragment using profiling information")),
97-
cl::values(clEnumValN(
98-
SplitFunctionsStrategy::Random2, "random2",
99-
"split each function into a hot and cold fragment at a randomly chosen "
100-
"split point (ignoring any available profiling information)")),
101-
cl::values(clEnumValN(
102-
SplitFunctionsStrategy::RandomN, "randomN",
103-
"split each function into N fragments at a randomly chosen split "
104-
"points (ignoring any available profiling information)")),
105-
cl::values(clEnumValN(
106-
SplitFunctionsStrategy::All, "all",
107-
"split all basic blocks of each function into fragments such that each "
108-
"fragment contains exactly a single basic block")),
109-
cl::desc("strategy used to partition blocks into fragments"),
110-
cl::cat(BoltOptCategory));
111-
11289
static cl::opt<double> CallScale(
11390
"call-scale",
11491
cl::desc("Call score scale coefficient (when --split-strategy=cdsplit)"),
@@ -724,14 +701,14 @@ Error SplitFunctions::runOnFunctions(BinaryContext &BC) {
724701
// If split strategy is not CDSplit, then a second run of the pass is not
725702
// needed after function reordering.
726703
if (BC.HasFinalizedFunctionOrder &&
727-
opts::SplitStrategy != SplitFunctionsStrategy::CDSplit)
704+
opts::SplitStrategy != opts::SplitFunctionsStrategy::CDSplit)
728705
return Error::success();
729706

730707
std::unique_ptr<SplitStrategy> Strategy;
731708
bool ForceSequential = false;
732709

733710
switch (opts::SplitStrategy) {
734-
case SplitFunctionsStrategy::CDSplit:
711+
case opts::SplitFunctionsStrategy::CDSplit:
735712
// CDSplit runs two splitting passes: hot-cold splitting (SplitPrfoile2)
736713
// before function reordering and hot-warm-cold splitting
737714
// (SplitCacheDirected) after function reordering.
@@ -742,21 +719,21 @@ Error SplitFunctions::runOnFunctions(BinaryContext &BC) {
742719
opts::AggressiveSplitting = true;
743720
BC.HasWarmSection = true;
744721
break;
745-
case SplitFunctionsStrategy::Profile2:
722+
case opts::SplitFunctionsStrategy::Profile2:
746723
Strategy = std::make_unique<SplitProfile2>();
747724
break;
748-
case SplitFunctionsStrategy::Random2:
725+
case opts::SplitFunctionsStrategy::Random2:
749726
Strategy = std::make_unique<SplitRandom2>();
750727
// If we split functions randomly, we need to ensure that across runs with
751728
// the same input, we generate random numbers for each function in the same
752729
// order.
753730
ForceSequential = true;
754731
break;
755-
case SplitFunctionsStrategy::RandomN:
732+
case opts::SplitFunctionsStrategy::RandomN:
756733
Strategy = std::make_unique<SplitRandomN>();
757734
ForceSequential = true;
758735
break;
759-
case SplitFunctionsStrategy::All:
736+
case opts::SplitFunctionsStrategy::All:
760737
Strategy = std::make_unique<SplitAll>();
761738
break;
762739
}

bolt/lib/Rewrite/RewriteInstance.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2115,6 +2115,13 @@ void RewriteInstance::adjustCommandLineOptions() {
21152115
opts::SplitEH = false;
21162116
}
21172117

2118+
if (BC->isAArch64() && !opts::CompactCodeModel &&
2119+
opts::SplitStrategy == opts::SplitFunctionsStrategy::CDSplit) {
2120+
BC->errs() << "BOLT-ERROR: CDSplit is not supported with LongJmp. Try with "
2121+
"'--compact-code-model'\n";
2122+
exit(1);
2123+
}
2124+
21182125
if (opts::StrictMode && !BC->HasRelocations) {
21192126
BC->errs()
21202127
<< "BOLT-WARNING: disabling strict mode (-strict) in non-relocation "

bolt/lib/Utils/CommandLineOpts.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,29 @@ ExecutionCountThreshold("execution-count-threshold",
104104
cl::Hidden,
105105
cl::cat(BoltOptCategory));
106106

107+
cl::opt<SplitFunctionsStrategy> SplitStrategy(
108+
"split-strategy", cl::init(SplitFunctionsStrategy::Profile2),
109+
cl::values(clEnumValN(SplitFunctionsStrategy::Profile2, "profile2",
110+
"split each function into a hot and cold fragment "
111+
"using profiling information")),
112+
cl::values(clEnumValN(SplitFunctionsStrategy::CDSplit, "cdsplit",
113+
"split each function into a hot, warm, and cold "
114+
"fragment using profiling information")),
115+
cl::values(clEnumValN(
116+
SplitFunctionsStrategy::Random2, "random2",
117+
"split each function into a hot and cold fragment at a randomly chosen "
118+
"split point (ignoring any available profiling information)")),
119+
cl::values(clEnumValN(
120+
SplitFunctionsStrategy::RandomN, "randomN",
121+
"split each function into N fragments at a randomly chosen split "
122+
"points (ignoring any available profiling information)")),
123+
cl::values(clEnumValN(
124+
SplitFunctionsStrategy::All, "all",
125+
"split all basic blocks of each function into fragments such that each "
126+
"fragment contains exactly a single basic block")),
127+
cl::desc("strategy used to partition blocks into fragments"),
128+
cl::cat(BoltOptCategory));
129+
107130
bool HeatmapBlockSpecParser::parse(cl::Option &O, StringRef ArgName,
108131
StringRef Arg, HeatmapBlockSizes &Val) {
109132
// Parses a human-readable suffix into a shift amount or nullopt on error.

bolt/test/AArch64/unsupported-passes.test

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
// REQUIRES: system-linux,asserts,target=aarch64{{.*}}
44

55
RUN: %clang %cflags %p/../Inputs/hello.c -o %t -Wl,-q
6-
RUN: not llvm-bolt %t -o %t.bolt --frame-opt=all 2>&1 | FileCheck %s
6+
RUN: not llvm-bolt %t -o %t.bolt --frame-opt=all 2>&1 | FileCheck %s --check-prefix=CHECK-FRAME-OPT
77

8-
CHECK: BOLT-ERROR: frame-optimizer is supported only on X86
8+
CHECK-FRAME-OPT: BOLT-ERROR: frame-optimizer is supported only on X86
9+
10+
RUN: not llvm-bolt %t -o %t.bolt split-functions --split-strategy=cdsplit 2>&1 | FileCheck %s --check-prefix=CHECK-CDSPLIT
11+
CHECK-CDSPLIT: BOLT-ERROR: CDSplit is not supported with LongJmp. Try with '--compact-code-model'

0 commit comments

Comments
 (0)