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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to