Address review comments

Author: aaupov

clang/CMakeLists.txt

@@ -853,19 +853,19 @@ endif()
 set(CLANG_BOLT "INSTRUMENT" CACHE STRING "Apply BOLT optimization to Clang. \
   May be specified as Instrument or Perf or LBR to use a particular profiling \
   mechanism.")
-string(TOUPPER "${CLANG_BOLT}" uppercase_CLANG_BOLT)
+string(TOUPPER "${CLANG_BOLT}" CLANG_BOLT)
 
 if (CLANG_BOLT AND NOT LLVM_BUILD_INSTRUMENTED)
   set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
   set(CLANG_INSTRUMENTED ${LLVM_RUNTIME_OUTPUT_INTDIR}/${CLANG_BOLT_INSTRUMENTED})
   set(BOLT_FDATA ${CMAKE_CURRENT_BINARY_DIR}/utils/perf-training/prof.fdata)
 
   # Pass extra flag in no-LBR mode
-  if (uppercase_CLANG_BOLT STREQUAL "PERF")
+  if (CLANG_BOLT STREQUAL "PERF")
     set(BOLT_NO_LBR "-nl")
   endif()
 
-  if (uppercase_CLANG_BOLT STREQUAL "INSTRUMENT")
+  if (CLANG_BOLT STREQUAL "INSTRUMENT")
     # Instrument clang with BOLT
     add_custom_target(clang-instrumented
       DEPENDS ${CLANG_INSTRUMENTED}

clang/utils/perf-training/CMakeLists.txt

@@ -84,13 +84,13 @@ if(CLANG_BOLT AND NOT LLVM_BUILD_INSTRUMENTED)
     COMMAND "${Python3_EXECUTABLE}" ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py clean ${CMAKE_CURRENT_BINARY_DIR} perf.data
     COMMENT "Clearing old perf data")
 
-  string(TOUPPER "${CLANG_BOLT}" uppercase_CLANG_BOLT)
-  if (uppercase_CLANG_BOLT STREQUAL "LBR")
+  string(TOUPPER "${CLANG_BOLT}" CLANG_BOLT)
+  if (CLANG_BOLT STREQUAL "LBR")
     set(BOLT_LBR "--lbr")
   endif()
 
   add_custom_target(merge-fdata-deps)
-  if (uppercase_CLANG_BOLT STREQUAL "INSTRUMENT")
+  if (CLANG_BOLT STREQUAL "INSTRUMENT")
     add_dependencies(merge-fdata-deps generate-bolt-fdata)
   else()
     # Convert perf profiles into fdata

clang/utils/perf-training/bolt.lit.cfg

@@ -6,20 +6,21 @@ import lit.util
 import os
 import subprocess
 
+clang_bolt_mode = config.clang_bolt_mode.lower()
 clang_binary = "clang"
-perf_wrapper = ""
-if config.clang_bolt_mode.lower() == "instrument":
+perf_wrapper = f"{config.python_exe} {config.perf_helper_dir}/perf-helper.py perf "
+
+if clang_bolt_mode == "instrument":
+    perf_wrapper = ""
     clang_binary = config.clang_bolt_name
-else:  # perf or LBR
-    perf_wrapper = "%s %s/perf-helper.py perf" % (
-        config.python_exe,
-        config.perf_helper_dir,
-    )
-    if config.clang_bolt_mode.lower() == "lbr":
-        perf_wrapper += " --lbr"
+elif clang_bolt_mode == "lbr":
+    perf_wrapper += " --lbr -- "
+elif clang_bolt_mode == "perf":
     perf_wrapper += " -- "
+else:
+    assert 0, "Unsupported CLANG_BOLT_MODE variable"
 
-config.clang = os.path.realpath(
+config.clang = perf_wrapper + os.path.realpath(
     lit.util.which(clang_binary, config.clang_tools_dir)
 ).replace("\\", "/")
 
@@ -42,16 +43,9 @@ config.suffixes = [
 use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")
 config.substitutions.append(
-    (
-        "%clang_cpp_skip_driver",
-        " %s %s --driver-mode=g++ " % (perf_wrapper, config.clang),
-    )
-)
-config.substitutions.append(
-    ("%clang_cpp", " %s %s --driver-mode=g++ " % (perf_wrapper, config.clang))
-)
-config.substitutions.append(
-    ("%clang_skip_driver", " %s %s " % (perf_wrapper, config.clang))
+    ("%clang_cpp_skip_driver", f" {config.clang} --driver-mode=g++ ")
 )
-config.substitutions.append(("%clang", " %s %s " % (perf_wrapper, config.clang)))
+config.substitutions.append(("%clang_cpp", f" {config.clang} --driver-mode=g++ "))
+config.substitutions.append(("%clang_skip_driver", config.clang))
+config.substitutions.append(("%clang", config.clang))
 config.substitutions.append(("%test_root", config.test_exec_root))

clang/utils/perf-training/perf-helper.py

@@ -74,15 +74,10 @@ def perf(args):
     parser.add_argument(
         "--lbr", action="store_true", help="Use perf with branch stacks"
     )
-    parser.add_argument("cmd", nargs="*", help="")
+    parser.add_argument("cmd", nargs=argparse.REMAINDER, help="")
 
-    # Use python's arg parser to handle all leading option arguments, but pass
-    # everything else through to perf
-    first_cmd = next(arg for arg in args if not arg.startswith("--"))
-    last_arg_idx = args.index(first_cmd)
-
-    opts = parser.parse_args(args[:last_arg_idx])
-    cmd = args[last_arg_idx:]
+    opts = parser.parse_args()
+    cmd = opts["cmd"][1:]
 
     perf_args = [
         "perf",