-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[BOLT] Add --ba flag to deprecate --nl #164257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The `--nl` flag, originally for Non-LBR mode, is deprecated and will be replaced by `--basic-events` (alias `--ba`). `--nl` remains as a deprecated alias for backward compatibility.
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-bolt Author: Paschalis Mpeis (paschalis-mpeis) ChangesThe
Full diff: https://github.com/llvm/llvm-project/pull/164257.diff 8 Files Affected:
diff --git a/bolt/README.md b/bolt/README.md
index bf52f8c416915..902d1eb6e7694 100644
--- a/bolt/README.md
+++ b/bolt/README.md
@@ -164,7 +164,7 @@ $ perf2bolt -p perf.data -o perf.fdata <executable>
This command will aggregate branch data from `perf.data` and store it in a
format that is both more compact and more resilient to binary modifications.
-If the profile was collected without brstacks, you will need to add `-nl` flag to
+If the profile was collected without brstacks, you will need to add `-ba` flag to
the command line above.
### Step 3: Optimize with BOLT
diff --git a/bolt/docs/Heatmaps.md b/bolt/docs/Heatmaps.md
index b10e72f0632e3..a9def71638465 100644
--- a/bolt/docs/Heatmaps.md
+++ b/bolt/docs/Heatmaps.md
@@ -21,7 +21,7 @@ $ perf record -e cycles:u -j any,u [-p PID|-a] -- sleep <interval>
```
Running with brstack (`-j any,u` or `-b`) is recommended. Heatmaps can be generated
-from basic events by using the llvm-bolt-heatmap option `-nl` (no brstack) but
+from basic events by using the llvm-bolt-heatmap option `-ba` (basic events) but
such heatmaps do not have the coverage provided by brstack and may only be useful
for finding event hotspots at larger code block granularities.
diff --git a/bolt/docs/index.rst b/bolt/docs/index.rst
index fce6d25500237..07cefd0476422 100644
--- a/bolt/docs/index.rst
+++ b/bolt/docs/index.rst
@@ -205,8 +205,8 @@ This command will aggregate branch data from ``perf.data`` and store it
in a format that is both more compact and more resilient to binary
modifications.
-If the profile was collected without LBRs, you will need to add ``-nl``
-flag to the command line above.
+If the profile was collected without brstacks, you will need to add `-ba` flag to
+the command line above.
Step 3: Optimize with BOLT
~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 9faccc23b4b81..a62fec60d685a 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -45,10 +45,18 @@ using namespace bolt;
namespace opts {
static cl::opt<bool>
- BasicAggregation("nl",
- cl::desc("aggregate basic samples (without brstack info)"),
+ BasicAggregation("basic-events",
+ cl::desc("aggregate basic events (without brstack info)"),
cl::cat(AggregatorCategory));
+static cl::alias BasicAggregationAlias("ba",
+ cl::desc("Alias for --basic-events"),
+ cl::aliasopt(BasicAggregation));
+
+static cl::alias BasicAggregationAliasNlDeprecated(
+ "nl", cl::desc("Alias for --basic-events (deprecated. Use --ba)"),
+ cl::aliasopt(BasicAggregation), llvm::cl::ReallyHidden);
+
cl::opt<bool> ArmSPE("spe", cl::desc("Enable Arm SPE mode."),
cl::cat(AggregatorCategory));
@@ -1433,7 +1441,7 @@ std::error_code DataAggregator::printLBRHeatMap() {
"Cannot build heatmap.";
} else {
errs() << "HEATMAP-ERROR: no brstack traces detected in profile. "
- "Cannot build heatmap. Use -nl for building heatmap from "
+ "Cannot build heatmap. Use -ba for building heatmap from "
"basic events.\n";
}
exit(1);
@@ -1629,8 +1637,8 @@ std::error_code DataAggregator::parseBranchEvents() {
<< "PERF2BOLT-WARNING: all recorded samples for this binary lack "
"brstack. Record profile with perf record -j any or run "
"perf2bolt "
- "in non-brstack mode with -nl (the performance improvement in "
- "-nl "
+ "in non-brstack mode with -ba (the performance improvement in "
+ "-ba "
"mode may be limited)\n";
else
errs()
diff --git a/bolt/test/X86/nolbr.s b/bolt/test/X86/nolbr.s
index d6710deeec343..432797eaaa129 100644
--- a/bolt/test/X86/nolbr.s
+++ b/bolt/test/X86/nolbr.s
@@ -10,7 +10,7 @@
# RUN: FileCheck %s --input-file %t.fdata --check-prefix=CHECK-FDATA
# RUN: llvm-strip --strip-unneeded %t.o
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -nostdlib
-# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --dyno-stats -nl \
+# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata --dyno-stats -ba \
# RUN: --print-only=_start 2>&1 | FileCheck %s --check-prefix=CHECK-BOLT
# CHECK-FDATA: no_lbr
diff --git a/bolt/test/X86/pre-aggregated-perf.test b/bolt/test/X86/pre-aggregated-perf.test
index c4f5b8f2ea445..6951d09db3de6 100644
--- a/bolt/test/X86/pre-aggregated-perf.test
+++ b/bolt/test/X86/pre-aggregated-perf.test
@@ -61,11 +61,11 @@ RUN: FileCheck %s -check-prefix=NEWFORMAT --input-file %t.bolt.yaml
RUN: perf2bolt %t.exe -o %t --pa -p %p/Inputs/pre-aggregated-basic.txt -o %t.ba \
RUN: 2>&1 | FileCheck %s --check-prefix=BASIC-ERROR
RUN: perf2bolt %t.exe -o %t --pa -p %p/Inputs/pre-aggregated-basic.txt -o %t.ba.nl \
-RUN: -nl 2>&1 | FileCheck %s --check-prefix=BASIC-SUCCESS
-RUN: FileCheck %s --input-file %t.ba.nl --check-prefix CHECK-BASIC-NL
+RUN: -ba 2>&1 | FileCheck %s --check-prefix=BASIC-SUCCESS
+RUN: FileCheck %s --input-file %t.ba.nl --check-prefix CHECK-BASIC-BA
BASIC-ERROR: BOLT-INFO: 0 out of 7 functions in the binary (0.0%) have non-empty execution profile
BASIC-SUCCESS: BOLT-INFO: 4 out of 7 functions in the binary (57.1%) have non-empty execution profile
-CHECK-BASIC-NL: no_lbr cycles
+CHECK-BASIC-BA: no_lbr cycles
PERF2BOLT: 1 frame_dummy/1 1e 1 frame_dummy/1 0 0 1
PERF2BOLT-NEXT: 1 main 451 1 SolveCubic 0 0 2
diff --git a/bolt/test/perf2bolt/perf_test.test b/bolt/test/perf2bolt/perf_test.test
index 08b3413a67a0b..984432716992a 100644
--- a/bolt/test/perf2bolt/perf_test.test
+++ b/bolt/test/perf2bolt/perf_test.test
@@ -4,7 +4,7 @@ REQUIRES: system-linux, perf
RUN: %clang %S/Inputs/perf_test.c -fuse-ld=lld -pie -Wl,--script=%S/Inputs/perf_test.lds -o %t
RUN: perf record -Fmax -e cycles:u -o %t2 -- %t
-RUN: perf2bolt %t -p=%t2 -o %t3 -nl -ignore-build-id --show-density \
+RUN: perf2bolt %t -p=%t2 -o %t3 -ba -ignore-build-id --show-density \
RUN: --heatmap %t.hm 2>&1 | FileCheck %s
RUN: FileCheck %s --input-file %t.hm-section-hotness.csv --check-prefix=CHECK-HM
@@ -15,7 +15,7 @@ CHECK: BOLT-INFO: Functions with density >= {{.*}} account for 99.00% total samp
RUN: %clang %S/Inputs/perf_test.c -no-pie -fuse-ld=lld -o %t4
RUN: perf record -Fmax -e cycles:u -o %t5 -- %t4
-RUN: perf2bolt %t4 -p=%t5 -o %t6 -nl -ignore-build-id --show-density \
+RUN: perf2bolt %t4 -p=%t5 -o %t6 -ba -ignore-build-id --show-density \
RUN: --heatmap %t.hm2 2>&1 | FileCheck %s
RUN: FileCheck %s --input-file %t.hm2-section-hotness.csv --check-prefix=CHECK-HM
diff --git a/clang/utils/perf-training/perf-helper.py b/clang/utils/perf-training/perf-helper.py
index 1c7904ec62163..97b771103ae2b 100644
--- a/clang/utils/perf-training/perf-helper.py
+++ b/clang/utils/perf-training/perf-helper.py
@@ -133,7 +133,7 @@ def perf2bolt(args):
"--profile-format=yaml",
]
if not opts.lbr:
- p2b_args += ["-nl"]
+ p2b_args += ["-ba"]
p2b_args += ["-p"]
for filename in findFilesWithExtension(opts.path, "perf.data"):
subprocess.check_call(p2b_args + [filename, "-o", filename + ".fdata"])
@@ -722,7 +722,7 @@ def bolt_optimize(args):
"-dyno-stats",
"-use-gnu-stack",
"-update-debug-sections",
- "-nl" if opts.method == "PERF" else "",
+ "-ba" if opts.method == "PERF" else "",
]
print("Running: " + " ".join(args))
process = subprocess.run(
|
|
LGTM |
aaupov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the handling for deprecated options (e.g. DeprecatedICFNumericOptionParser or DeprecatedSplitFunctionOptionParser) to warn the user about the old flag.
|
Thanks Amir. I've provided a callback instead for the warning, similar to I'll merge in a day or two if there are no further comments. |
The `--nl` flag, originally for Non-LBR mode, is deprecated and will be replaced by `--basic-events` (alias `--ba`). `--nl` remains as a deprecated alias for backward compatibility.
The `--nl` flag, originally for Non-LBR mode, is deprecated and will be replaced by `--basic-events` (alias `--ba`). `--nl` remains as a deprecated alias for backward compatibility.
The `--nl` flag, originally for Non-LBR mode, is deprecated and will be replaced by `--basic-events` (alias `--ba`). `--nl` remains as a deprecated alias for backward compatibility.
The `--nl` flag, originally for Non-LBR mode, is deprecated and will be replaced by `--basic-events` (alias `--ba`). `--nl` remains as a deprecated alias for backward compatibility.
The
--nlflag, originally for Non-LBR mode, is deprecated and will be replaced by--basic-events(alias--ba).--nlremains as a deprecated alias for backward compatibility.