phosek added inline comments.
================
Comment at: clang/utils/perf-training/bolt.lit.cfg:12
+if config.clang_bolt_mode.lower() == "instrument":
+ clang_binary = "clang-bolt.inst"
+else: # perf or LBR
----------------
This name would ideally be passed through the generated `lit.site.cfg` rather
than hardcoding it here.
================
Comment at: clang/utils/perf-training/perf-helper.py:75
+ parser.add_argument(
+ "--lbr", required=False, action="store_true", help="Use perf with
branch stacks"
+ )
----------------
This could be omitted.
================
Comment at: clang/utils/perf-training/perf-helper.py:85
+ opts = parser.parse_args(args[:last_arg_idx])
+ # cmd = shlex.split(args[last_arg_idx:])
+ cmd = args[last_arg_idx:]
----------------
Can we remove this?
================
Comment at: clang/utils/perf-training/perf-helper.py:88-97
+ perf_args = []
+ perf_args.extend(
+ (
+ "perf",
+ "record",
+ "--event=cycles:u",
+ "--freq=max",
----------------
Why not assign to `perf_args` directly?
================
Comment at: clang/utils/perf-training/perf-helper.py:115
+ )
+ parser.add_argument("p2b_path", help="Path to llvm-bolt")
+ parser.add_argument("path", help="Path containing perf.data files")
----------------
I'd call it `llvm-bolt` for simplicity.
================
Comment at: clang/utils/perf-training/perf-helper.py:118-120
+ parser.add_argument(
+ "--nolbr", required=False, action="store_true", help="Use -nl
perf2bolt mode"
+ )
----------------
I'd consider providing both `--lbr` and `--no-lbr` options.
================
Comment at: clang/utils/perf-training/perf-helper.py:123-126
+ p2b_args = []
+ p2b_args.extend(
+ (opts.p2b_path, opts.binary, "--aggregate-only",
"--profile-format=yaml")
+ )
----------------
Why not assign to `p2b_args` directly?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143617/new/
https://reviews.llvm.org/D143617
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
