[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py
Romain-Geissler-1A added a comment. Thank you ! Since I have no write access to the repository, can anyone of you commit this patch ? Cheers, Romain Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54071/new/ https://reviews.llvm.org/D54071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py
Romain-Geissler-1A created this revision. Romain-Geissler-1A added reviewers: rsmith, vsk, beanz. Herald added a subscriber: cfe-commits. Romain-Geissler-1A edited the summary of this revision. 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 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 Repository: rC Clang https://reviews.llvm.org/D54071 Files: utils/perf-training/perf-helper.py Index: utils/perf-training/perf-helper.py === --- utils/perf-training/perf-helper.py +++ utils/perf-training/perf-helper.py @@ -114,7 +114,7 @@ # 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. @@ -296,7 +296,7 @@ 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): @@ -340,7 +340,7 @@ # 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 Index: utils/perf-training/perf-helper.py === --- utils/perf-training/perf-helper.py +++ utils/perf-training/perf-helper.py @@ -114,7 +114,7 @@ # 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. @@ -296,7 +296,7 @@ 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): @@ -340,7 +340,7 @@ # 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40218: [Clang] Add __builtin_launder
Romain-Geissler-1A added a comment. Hi, Is there any news on this code review ? Is it ready to land ? Cheers, Romain https://reviews.llvm.org/D40218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility
Romain-Geissler-1A created this revision. Romain-Geissler-1A added reviewers: serge-sans-paille, chandlerc. Herald added a project: clang. Herald added a subscriber: cfe-commits. Hi, This simple patch ignores -fsemantic-interposition/-fno-semantic-interposition that may be used by some gcc users, by copy/pasting what was done for other similar -f flags. Cheers, Romain Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65616 Files: clang/include/clang/Driver/Options.td clang/test/Driver/clang_f_opts.c Index: clang/test/Driver/clang_f_opts.c === --- clang/test/Driver/clang_f_opts.c +++ clang/test/Driver/clang_f_opts.c @@ -298,6 +298,7 @@ // RUN: -fno-implement-inlines -fimplement-inlines\ // RUN: -fstack-check \ // RUN: -fforce-addr \ +// RUN: -fno-semantic-interposition \ // RUN: -malign-functions=100 \ // RUN: -malign-loops=100 \ // RUN: -malign-jumps=100 \ Index: clang/include/clang/Driver/Options.td === --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -3153,6 +3153,7 @@ defm devirtualize : BooleanFFlag<"devirtualize">, Group; defm devirtualize_speculatively : BooleanFFlag<"devirtualize-speculatively">, Group; +defm semantic_interposition : BooleanFFlag<"semantic-interposition">, Group; // Generic gfortran options. def A_DASH : Joined<["-"], "A-">, Group; Index: clang/test/Driver/clang_f_opts.c === --- clang/test/Driver/clang_f_opts.c +++ clang/test/Driver/clang_f_opts.c @@ -298,6 +298,7 @@ // RUN: -fno-implement-inlines -fimplement-inlines\ // RUN: -fstack-check \ // RUN: -fforce-addr \ +// RUN: -fno-semantic-interposition \ // RUN: -malign-functions=100 \ // RUN: -malign-loops=100 \ // RUN: -malign-jumps=100 \ Index: clang/include/clang/Driver/Options.td === --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -3153,6 +3153,7 @@ defm devirtualize : BooleanFFlag<"devirtualize">, Group; defm devirtualize_speculatively : BooleanFFlag<"devirtualize-speculatively">, Group; +defm semantic_interposition : BooleanFFlag<"semantic-interposition">, Group; // Generic gfortran options. def A_DASH : Joined<["-"], "A-">, Group; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility
Romain-Geissler-1A added a comment. Hi, Thanks Serge. Given that I am only a very occasional contributor, I don't have any right to push this commit myself. Would you please commit it for me ? Thanks, Romain Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65616/new/ https://reviews.llvm.org/D65616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility
Romain-Geissler-1A added a comment. Hi, The only group I know which uses -fsemantic-interposition is the company I work for: Amadeus. Since for now the flag doesn't exist on clang, no we don't know that the future clang implementation doesn't match the gcc one ;) But then we have to wonder if clang wants to reuse the same name if that doesn't do the same thing if really the differences are big. What are the differences between the gcc and the LLVM implementation (when -fsemantic-interposition will be wired up in clang) ? For now we are not using clang/llvm tools for anything else than static analysis and the useful tools from clang-extra-tools. In other words, we don't care about code gen of LLVM, yet (that may come eventually). My immediate concern right now is that when you use clang-tidy and friends with a compilation database generated for gcc, then you we end-up with warnings for each of the non-usual flags we use in Amadeus, like for example -fsemantic-interposition, that "pollutes" the output of these tools for each translation unit and thus it brings a user unfriendly experience. I am fine with wiring up a new flag -fsemantic-interposition in clang, however I have no idea what should be done for that, I have absolutely 0 knowledge in the LLVM source code, yet. Cheers, Romain Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65616/new/ https://reviews.llvm.org/D65616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py
Romain-Geissler-1A added subscribers: michaelplatings, serge-sans-paille. Romain-Geissler-1A added a comment. Herald added a reviewer: serge-sans-paille. 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 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54071/new/ https://reviews.llvm.org/D54071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py
Romain-Geissler-1A updated this revision to Diff 181140. Romain-Geissler-1A added a comment. Rebased to master Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54071/new/ https://reviews.llvm.org/D54071 Files: utils/perf-training/perf-helper.py Index: utils/perf-training/perf-helper.py === --- utils/perf-training/perf-helper.py +++ utils/perf-training/perf-helper.py @@ -114,7 +114,7 @@ # 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. @@ -340,7 +340,7 @@ # 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 Index: utils/perf-training/perf-helper.py === --- utils/perf-training/perf-helper.py +++ utils/perf-training/perf-helper.py @@ -114,7 +114,7 @@ # 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. @@ -340,7 +340,7 @@ # 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards
Romain-Geissler-1A added a comment. Herald added a subscriber: steakhal. Hi, Is there any update about this ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits