Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
On 8/17/23 3:54 AM, Richard Biener wrote: I think it needs a new category, 'inline' is probably the "closest" existing one but that also tends to be noisy. Maybe 'call' would be a good name? We could report things like tail-recursion optimization, tail-calling and sibling calling optimizations there, possibly also return/argument copy elision. OK, thanks. I have two questions: 1. Is the information dumped by -fopt-info intended for compiler developers, to see something of the internal logic of gcc, or for end users? 2. You say that "'inline' ... tends to be noisy". Most of the output I see from -fopt-info-missed is basically _io.c:103829:4: missed: not inlinable: ___H___io/396 -> __builtin_expect/2486, function body not available Is ___builtin_expect truly a function whose body is not available, or should -fopt-info-missed not report these instances? Brad
Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
First, if this is no longer the appropriate group for this discussion, please tell me where to send it. I've been working to understand all the comments here. From them, I think: 1. It's OK to have gcc report back to the user whether each particular call in tail position is optimized when -foptimize-sibling-calls is set as a compiler option; or, to report only those calls that have not been optimized. 2. Given (1), the question is what form that information should take, and which gcc option should cause it to be expressed. From comments in this thread and the documentation for today's mainline gcc, I configured and built Gambit Scheme with ./configure CC="/pkgs/gcc-mainline/bin/gcc -fopt-info-missed" --enable-single-host thinking that info about missed optimizations would be a good place to export information about non-optimized sibling calls. This may not have been a good idea, however, as I ended up with 93367 lines about missed optimizations. Is this the right direction to proceed in? The documentation says about -fopt-info-missed One or more of the following option keywords can be used to describe a group of optimizations: 'ipa' Enable dumps from all interprocedural optimizations. 'loop' Enable dumps from all loop optimizations. 'inline' Enable dumps from all inlining optimizations. 'omp' Enable dumps from all OMP (Offloading and Multi Processing) optimizations. 'vec' Enable dumps from all vectorization optimizations. 'optall' Enable dumps from all optimizations. This is a superset of the optimization groups listed above. I'd like to limit the number of missed optimization warnings, but I don't know where sibling call optimization would fit into these categories. Brad
Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
Thank you for your comments. I have a few questions. I don't think this specific case qualifies for -Wdisabled-optimization. The diagnostic is for cases the user can control and was invented for limits we put up for compile-time and memory-usage issues where there exist --param XYZ to adjust limits. It would be more appropriate to change this to dump_printf_loc (MSG_MISSED_OPTIMIZATION, ...) where this was designe to diagnose cases the compiler failed to optimize for other reasons than running into some --param. I'm sorry, I don't understand what dump_printf_loc does, where does it dump this information? What is the form of information that is usually dumped, and for which purpose? So, NAK. What does "NAK" mean? When I added the -Wdisabled-optimization warning to gcc in 2000, I was trying to give the user information about when the compiler may not do an optimization that the user asks for. And yes, my idea was that the user can either ignore the warning or do something to the code or change a --param to allow the optimization to succeed. Whether gcc actually optimizes tail or sibling calls that appear in the Gambit source code has been a point of discussion among the Gambit Scheme community for years; sometimes that discussion has bled into the GCC mail lists, there's a fairly long thread here: https://gcc.gnu.org/pipermail/gcc-help/2021-December/140957.html I actually built a profiled gcc to see whether maybe_complain_about_tail_call was called when compiling the output of Gambit's Scheme->C compiler; afterwards I reported to the Gambit mail list that it had not, all sibling calls were optimized. To me, -Wdisabled-optimization seems very appropriate when the user asks explicitly for -foptimize-sibling-calls (which the Gambit makefile does, because it doesn't want all -O2 optimizations) and a sibling call can't be optimized. maybe_complain_about_tail_call is even passed a helpful message that explains *why* the call can't be optimized. With this information, the user can actually do something, rewrite the code to remove the obstacle, or decide that optimizing that particular call isn't important (which sometimes it isn't, if it's in the handwritten C support library instead of the C code generated by the Scheme->C compiler). So I'm really hoping that some kind of information can be sent back to the user in this situation. Brad
Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
On 8/5/23 5:53 PM, David Malcolm wrote: ...but the warning branch uses "warning", which implicitly uses the input_location global variable. Is the warning reported at the correct place? It's better to use warning_at and pass it the location at which the warning should be emitted. Thanks, I changed the patch to follow your suggestion. I built and ran make check with the patch; there were no changes to the test results. As a test, I again built GCC with ../../gcc-mainline/configure CXX="/pkgs/gcc-mainline-new-new/bin/g++ -Wdisabled-optimization" --enable-languages=c --disable-multilib --prefix=/pkgs/gcc-mainline-test-test --disable-werror --disable-bootstrap I found no changes to the warning messages. Brad diff --git a/gcc/calls.cc b/gcc/calls.cc index 1f3a6d5c450..de293ac51bb 100644 --- a/gcc/calls.cc +++ b/gcc/calls.cc @@ -1242,10 +1242,12 @@ void maybe_complain_about_tail_call (tree call_expr, const char *reason) { gcc_assert (TREE_CODE (call_expr) == CALL_EXPR); - if (!CALL_EXPR_MUST_TAIL_CALL (call_expr)) -return; - - error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason); + if (CALL_EXPR_MUST_TAIL_CALL (call_expr)) +error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason); + else if (flag_optimize_sibling_calls) +warning_at (EXPR_LOCATION (call_expr), OPT_Wdisabled_optimization, +"cannot apply sibling-call optimization: %s", reason); + return; } /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
On 8/5/23 4:58 PM, Prathamesh Kulkarni wrote: I don't have comments on the patch, but a new warning will also require a corresponding entry in doc/invoke.texi. Thank you for your comment. -Wdisabled-optimization is an established warning, it's just that I'd like it to apply in another circumstance. Maybe that doesn't need new documentation. Brad Lucier
[PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls
The patch at the end adds a warning when a tail/sibling call cannot be optimized for various reasons. I built and tested GCC with and without the patch with configuration Configured with: ../../gcc-mainline/configure --enable-languages=c --disable-multilib --prefix=/pkgs/gcc-mainline --disable-werror There were some changes in the test results, but I can't say that they look substantive: diff -C 2 summary.log ../gcc-mainline *** summary.log Thu Aug 3 22:56:13 2023 --- ../gcc-mainline/summary.log Thu Aug 3 19:42:33 2023 *** *** 14,22 === g++ Summary === ! # of expected passes 239234 # of unexpected failures 5 # of expected failures2087 ! # of unsupported tests10566 ! /home/lucier/programs/gcc/objdirs/gcc-mainline-new/gcc/xg++ version 14.0.0 20230802 (experimental) (GCC) === gcc tests === --- 14,22 === g++ Summary === ! # of expected passes 239262 # of unexpected failures 5 # of expected failures2087 ! # of unsupported tests10562 ! /home/lucier/programs/gcc/objdirs/gcc-mainline/gcc/xg++ version 14.0.0 20230802 (experimental) (GCC) === gcc tests === *** *** 155,164 === gcc Summary === ! # of expected passes 192553 # of unexpected failures 109 # of unexpected successes 19 # of expected failures1506 ! # of unsupported tests2623 ! /home/lucier/programs/gcc/objdirs/gcc-mainline-new/gcc/xgcc version 14.0.0 20230802 (experimental) (GCC) === libatomic tests === --- 155,164 === gcc Summary === ! # of expected passes 192563 # of unexpected failures 109 # of unexpected successes 19 # of expected failures1506 ! # of unsupported tests2619 ! /home/lucier/programs/gcc/objdirs/gcc-mainline/gcc/xgcc version 14.0.0 20230802 (experimental) (GCC) === libatomic tests === I then configured and built GCC with ../../gcc-mainline/configure CXX="/pkgs/gcc-mainline-new/bin/g++ -Wdisabled-optimization" --enable-languages=c --disable-multilib --prefix=/pkgs/gcc-mainline-test --disable-werror --disable-bootstrap to test the new warning. The warnings are of the form, e.g., ../../../gcc-mainline/gcc/tree-vect-stmts.cc:11990:44: warning: cannot apply sibling-call optimization: callee required more stack slots than the caller [-Wdisabled-optimization] These are the number of times this warning was triggered building stage1: grep warning: build.log | grep sibling | sed 's/^.*://' | sort | uniq -c 259 callee required more stack slots than the caller [-Wdisabled-optimization] 43 callee returns a structure [-Wdisabled-optimization] If this patch is OK, someone else will need to commit it for me. Brad gcc/Changelog * calls.cc (maybe_complain_about_tail_call) Add warning when tail or sibling call cannot be optimized. diff --git a/gcc/calls.cc b/gcc/calls.cc index 1f3a6d5c450..b95c876fda8 100644 --- a/gcc/calls.cc +++ b/gcc/calls.cc @@ -1242,10 +1242,12 @@ void maybe_complain_about_tail_call (tree call_expr, const char *reason) { gcc_assert (TREE_CODE (call_expr) == CALL_EXPR); - if (!CALL_EXPR_MUST_TAIL_CALL (call_expr)) -return; - - error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason); + if (CALL_EXPR_MUST_TAIL_CALL (call_expr)) +error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason); + else if (flag_optimize_sibling_calls) +warning (OPT_Wdisabled_optimization, + "cannot apply sibling-call optimization: %s", reason); + return; } /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
Re: Where to place warning about non-optimized tail and sibling calls
On 8/1/23 6:08 PM, David Malcolm wrote: FWIW I added it to support Scheme from libgccjit; Do you know of any Scheme using libgccjit? BTW, I tried to build mainline with --enable-coverage to see which code is executed with -foptimize-sibling-calls, but bootstrap fails with /home/lucier/programs/gcc/objdirs/gcc-mainline/./prev-gcc/xg++ -B/home/lucier/programs/gcc/objdirs/gcc-mainline/./prev-gcc/ -B/pkgs/gcc-mainline/x86_64-pc-linux-gnu/bin/ -nostdinc++ -B/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -B/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -I/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/include -I/home/lucier/programs/gcc/gcc-mainline/libstdc++-v3/libsupc++ -L/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/src/.libs -L/home/lucier/programs/gcc/objdirs/gcc-mainline/prev-x86_64-pc-linux-gnu/libstdc++-v3/libsupc++/.libs -fno-PIE -c -g -O2 -fno-checking -gtoggle -DIN_GCC -fprofile-arcs -ftest-coverage -frandom-seed=opts.o -O0 -fkeep-static-functions -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I../../../gcc-mainline/gcc -I../../../gcc-mainline/gcc/. -I../../../gcc-mainline/gcc/../include -I../../../gcc-mainline/gcc/../libcpp/include -I../../../gcc-mainline/gcc/../libcody -I../../../gcc-mainline/gcc/../libdecnumber -I../../../gcc-mainline/gcc/../libdecnumber/bid -I../libdecnumber -I../../../gcc-mainline/gcc/../libbacktrace -o opts.o -MT opts.o -MMD -MP -MF ./.deps/opts.TPo ../../../gcc-mainline/gcc/opts.cc ../../../gcc-mainline/gcc/opts.cc: In function 'void print_filtered_help(unsigned int, unsigned int, unsigned int, unsigned int, gcc_options*, unsigned int)': ../../../gcc-mainline/gcc/opts.cc:1687:26: error: ' ' directive output may be truncated writing 2 bytes into a region of size between 1 and 256 [-Werror=format-truncation=] 1687 | "%s %s", help, _(use_diagnosed_msg)); | ^~ ../../../gcc-mainline/gcc/opts.cc:1686:22: note: 'snprintf' output 3 or more bytes (assuming 258) into a destination of size 256 1686 | snprintf (new_help, sizeof new_help, | ~^~~ 1687 | "%s %s", help, _(use_diagnosed_msg)); | ~ cc1plus: all warnings being treated as errors
Re: Where to place warning about non-optimized tail and sibling calls
On 8/1/23 6:08 PM, David Malcolm wrote: Or from libgccjit. FWIW I added it to support Scheme from libgccjit; see this patch kit: https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01287.html Perhaps there's a case for a frontend attribute for this. Dave Thanks. I thought a front-end warning might be enough, as one can add -Werror to the command line to fail if it can't optimize a sibling call. Is there a reasonable place to put a warning if flag_optimize_sibling_calls is true and a sibling call is *not* optimized? Brad
Re: Where to place warning about non-optimized tail and sibling calls
On 8/1/23 12:51 PM, Paul Koning wrote: How is it possible to write valid C that is correct only if some optimization is done? Perhaps "incorrect" was the wrong word. If sibling-call optimization is not done, then perhaps the program will blow out the stack, which would not happen if the optimization is done. Also, transforming sibling calls is an optimization for C, but for Scheme it's a part of the language. Translating Scheme to C has to take that into account: if there is no sibling-call optimization, then the Scheme code is translated to C code that uses a so-called trampoline; if there is sibling-call optimization that the Scheme compiler can rely on, then taking advantage of the optimization leads to faster Scheme code. Brad
Where to place warning about non-optimized tail and sibling calls
The Gambit Scheme->C compiler has an option to generate more efficient code if it knows that all tail and sibling calls in the generated C code will be optimized. If gcc does not, however, optimize a tail or sibling call, the generated C code may be incorrect (depending on circumstances). So I would like to add a warning enabled by -Wdisabled-optimization so that if -foptimize-sibling-calls is given and a tail or sibling call is not optimized, then a warning is triggered. I don't quite know where to place the warning. It would be good if there were one piece of code to identify all tail and sibling calls, and then another piece that decides whether the optimization can be performed. I see code in gcc/tree-tailcall.cc suitable_for_tail_opt_p suitable_for_tail_call_opt_p which are called by tree_optimize_tail_calls_1 which takes an argument opt_tailcalls and it's called in one place with opt_tailcalls true and in another place with opt_tailcalls false. So I'm losing the plot here. There is other code dealing with tail calls in gcc/calls.cc I don't seem to understand at all. Any advice? Brad