[Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

[Romain-Geissler-1A]

Summary

Hi,

Current clang fail to bootstrap in PGO mode when only python3 is available, because perf-helper.py is not compatible with python 3. We can see two errors:

  File "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py", line 299
    by_count.sort(key = lambda (_,n): -n)
                               ^
SyntaxError: invalid syntax

which can be fixed by removing the parameter parenthesis.

And

$ "/usr/bin/python3.6" "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py" "cc1" "/workdir/build/final-system/llvm-build/tools/clang/stage2-instrumented-bins/bin/clang" "-Wall" "-pedantic" "-c" "/home/jenkins/workspace/OTF_Toolchain_release_2.0-HLXHYRKCVDYQJLF23VGZ3MVAU6VGURX537LUE3KFVM2SSPMZ6IOA/output/src/llvm-7.0.0.src/tools/clang/utils/perf-training/cxx/hello_world.cpp"
# command stderr:
Traceback (most recent call last):
  File "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py", line 408, in <module>
    main()
  File "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py", line 405, in main
    sys.exit(f(sys.argv[2:]))
  File "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py", line 159, in cc1
    cc1_cmd = get_cc1_command_for_args(cmd, cc1_env)
  File "/workdir/src/llvm-7.0.0.src/tools/clang/utils/perf-training/perf-helper.py", line 119, in get_cc1_command_for_args
    for ln in cc_output.split('\n'):
TypeError: a bytes-like object is required, not 'str'

error: command failed with exit status: 1

which can be made both 2 and 3 compatible by using the attribute universal_newlines=True

Cheers,
Romain

[Romain-Geissler-1A]

Hi @serge-sans-paille @michaelplatings,

I saw you recently merged some Python 3 related stuff. Here is an older review from November still never reviewed. You fixed already the fix around the lambda, yet I still need to add "universal_newlines" on current trunk otherwise I have some bytes vs str objects error. Do you think you can land this small change too ?

Cheers,
Romain

[serge-sans-paille]

@Romain-Geissler-1A sure thing. Can you rebase your patch on master first?

[Romain-Geissler-1A]

Rebased to master

[serge-sans-paille]

LGTM.

[Romain-Geissler-1A]

Thank you !

Since I have no write access to the repository, can anyone of you commit this patch ?

Cheers,
Romain

utils/perf-training/perf-helper.py

def dtrace(args):
   print("... data collection took %.4fs" % elapsed)
 
   return 0
 
 def get_cc1_command_for_args(cmd, env):
   # Find the cc1 command used by the compiler. To do this we execute the
   # compiler with '-###' to figure out what it wants to do.
   cmd = cmd + ['-###']
-  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env).strip()
+  cc_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env, universal_newlines=True).strip()
   cc_commands = []
   for ln in cc_output.split('\n'):
       # Filter out known garbage.
       if (ln == 'Using built-in specs.' or
           ln.startswith('Configured with:') or
           ln.startswith('Target:') or
           ln.startswith('Thread model:') or
           ln.startswith('InstalledDir:') or
def form_by_frequency(symbol_lists):
   # first. This assumes the data files didn't use the oneshot dtrace method.
  
   counts = {}
   for symbols in symbol_lists:
     for a in symbols:
       counts[a] = counts.get(a,0) + 1
 
   by_count = counts.items()
-  by_count.sort(key = lambda (_,n): -n)
+  by_count.sort(key = lambda _,n: -n)
   return [s for s,n in by_count]
  
 def form_by_random(symbol_lists):
   # Randomize the symbols.
   merged_symbols = uniq(s for symbols in symbol_lists
                           for s in symbols)
   random.shuffle(merged_symbols)
   return merged_symbols
def genOrderFile(args):
   parser.add_argument("--method", dest="method",
     help="order file generation method to use", choices=methods.keys(),
     default='call_order')
   opts = parser.parse_args(args)
 
   # If the user gave us a binary, get all the symbols in the binary by
   # snarfing 'nm' output.
   if opts.binary_path is not None:
-     output = subprocess.check_output(['nm', '-P', opts.binary_path])
+     output = subprocess.check_output(['nm', '-P', opts.binary_path], universal_newlines=True)
      lines = output.split("\n")
      all_symbols = [ln.split(' ',1)[0]
                     for ln in lines
                     if ln.strip()]
      print("found %d symbols in binary" % len(all_symbols))
      all_symbols.sort()
   else:
      all_symbols = []