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