[PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Bradley Lucier]

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 failures		2087
! # of unsupported tests		10566
! /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 failures		2087
! # of unsupported tests		10562
! /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 failures		1506
! # of unsupported tests		2623
! /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 failures		1506
! # of unsupported tests		2619
! /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: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Prathamesh Kulkarni]

On Fri, 4 Aug 2023 at 23:28, Bradley Lucier via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> 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 failures               2087
> ! # of unsupported tests                10566
> ! /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 failures               2087
> ! # of unsupported tests                10562
> ! /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 failures               1506
> ! # of unsupported tests                2623
> ! /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 failures               1506
> ! # of unsupported tests                2619
> ! /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.
Hi Bradley,
I don't have comments on the patch, but a new warning will also
require a corresponding entry in doc/invoke.texi.

Thanks,
Prathamesh
>
> 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: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Bradley Lucier]

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

Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Prathamesh Kulkarni]

On Sun, 6 Aug 2023 at 03:07, Bradley Lucier <lucier@purdue.edu> wrote:
>
> 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.
Oops I misread your patch as adding a new warning :/
Sorry for the noise.

Best Regards,
Prathamesh
>
> Brad Lucier

Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[David Malcolm]

On Sun, 2023-08-06 at 02:28 +0530, Prathamesh Kulkarni via Gcc-patches
wrote:
> On Fri, 4 Aug 2023 at 23:28, Bradley Lucier via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:

Hi Bradley and Prathamesh...

> > 
> > 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:
> > 

[...]

> > 
> > 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.
> Hi Bradley,
> I don't have comments on the patch, but a new warning will also
> require a corresponding entry in doc/invoke.texi.

To nitpick, this isn't a new warning; the patch is extending an
existing warning.  Looking at the existing entry for that warning I
see:

@opindex Wdisabled-optimization
@opindex Wno-disabled-optimization
@item -Wdisabled-optimization
Warn if a requested optimization pass is disabled.  This warning does
not generally indicate that there is anything wrong with your code; it
merely indicates that GCC's optimizers are unable to handle the code
effectively.  Often, the problem is that your code is too big or too
complex; GCC refuses to optimize programs when the optimization
itself is likely to take inordinate amounts of time.

...which arguably fits the new functionality.  Though I don't know how
the optimizer maintainers feel about it.  Also, as we add more stuff to
this warning, would users need more fine-grained control over which
things for the optimizer to complain about?  I'm not sure.

> 
> Thanks,
> Prathamesh
> > 
> > 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);

The existing code use error_at, passing it the location of the
call_expr...

> > +  else if (flag_optimize_sibling_calls)
> > +    warning (OPT_Wdisabled_optimization,
> > +             "cannot apply sibling-call optimization: %s",
> > reason);

...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.

The patch doesn't add any test cases, but I imagine any such cases
would be very target-dependent (did I add any to my libgccjit version
of this way back when?)

Thanks for the patch; hope this is constructive
Dave



> > +  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

[Bradley Lucier]

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

[Richard Biener]

On Sat, Aug 5, 2023 at 11:54 PM David Malcolm via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Sun, 2023-08-06 at 02:28 +0530, Prathamesh Kulkarni via Gcc-patches
> wrote:
> > On Fri, 4 Aug 2023 at 23:28, Bradley Lucier via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
>
> Hi Bradley and Prathamesh...
>
> > >
> > > 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:
> > >
>
> [...]
>
> > >
> > > 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.
> > Hi Bradley,
> > I don't have comments on the patch, but a new warning will also
> > require a corresponding entry in doc/invoke.texi.
>
> To nitpick, this isn't a new warning; the patch is extending an
> existing warning.  Looking at the existing entry for that warning I
> see:
>
> @opindex Wdisabled-optimization
> @opindex Wno-disabled-optimization
> @item -Wdisabled-optimization
> Warn if a requested optimization pass is disabled.  This warning does
> not generally indicate that there is anything wrong with your code; it
> merely indicates that GCC's optimizers are unable to handle the code
> effectively.  Often, the problem is that your code is too big or too
> complex; GCC refuses to optimize programs when the optimization
> itself is likely to take inordinate amounts of time.
>
> ...which arguably fits the new functionality.  Though I don't know how
> the optimizer maintainers feel about it.  Also, as we add more stuff to
> this warning, would users need more fine-grained control over which
> things for the optimizer to complain about?  I'm not sure.

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.

So, NAK.

Thanks,
Richard.

> >
> > Thanks,
> > Prathamesh
> > >
> > > 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);
>
> The existing code use error_at, passing it the location of the
> call_expr...
>
> > > +  else if (flag_optimize_sibling_calls)
> > > +    warning (OPT_Wdisabled_optimization,
> > > +             "cannot apply sibling-call optimization: %s",
> > > reason);
>
> ...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.
>
> The patch doesn't add any test cases, but I imagine any such cases
> would be very target-dependent (did I add any to my libgccjit version
> of this way back when?)
>
> Thanks for the patch; hope this is constructive
> Dave
>
>
>
> > > +  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

[Bradley Lucier]

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

[Richard Biener]

On Mon, Aug 7, 2023 at 9:04 PM Bradley Lucier <lucier@purdue.edu> wrote:
>
> 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?

dump_printf_loc is able to direct the information to multiple places,
for one it amends the IL/pass dump file produced with -fdump-tree-<passname>,
but it also produces information for the -fopt-info* family of switches.
For example -fopt-info-vec produces a set of vectorized locations
as diagnostics, -fopt-info-vec-missed a set of diagnostics related to
missed vectorization opportunities.  GCC recently gained the ability to
format some of the diagnostics as SARIF, I'm not sure if -fopt-info related
diagnostics are covered, but at least -fsave-optimization-record produces
JSON output.

So we now have a better and more general machinery to diagnose
optimized/missed optimized things for passes than -Wdisabled-optimization
and _disabled_ optimization should now be taken literally when the optimization
was disabled by the user via a (default) choice of a --param value for example
(or a conflicting different optimization option).

The granularity for reporting -fopt-info is optimization groups (see
dumpfile.h, the OPTGROUP_* enum), I believe maybe_complain_about_tail_call
invocations are all from RTL expansion which currently is
OPTGROUP_NONE which would have to change (I think it's currently
not possible to explicitely specify an alternate optgroup at dump_printf time
(David?).

So the simplest way forward would be to add, say, OPTGROUP_EXPAND,
use dump_printf in maybe_complain_about_tail_call (which will also
dump info to the .expand debug dump which looks useful) and recognize
-fopt-info-expand.

> > So, NAK.
>
> What does "NAK" mean?

not acknowledged

> 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

[David Malcolm]

On Tue, 2023-08-08 at 08:05 +0200, Richard Biener wrote:
> On Mon, Aug 7, 2023 at 9:04 PM Bradley Lucier <lucier@purdue.edu>
> wrote:
> > 
> > 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?
> 
> dump_printf_loc is able to direct the information to multiple places,
> for one it amends the IL/pass dump file produced with -fdump-tree-
> <passname>,
> but it also produces information for the -fopt-info* family of
> switches.
> For example -fopt-info-vec produces a set of vectorized locations
> as diagnostics, -fopt-info-vec-missed a set of diagnostics related to
> missed vectorization opportunities.  GCC recently gained the ability
> to
> format some of the diagnostics as SARIF, I'm not sure if -fopt-info
> related
> diagnostics are covered, 

FWIW the SARIF output doesn't contain the -fopt-info stuff.

> but at least -fsave-optimization-record produces
> JSON output.
> 
> So we now have a better and more general machinery to diagnose
> optimized/missed optimized things for passes than -Wdisabled-
> optimization
> and _disabled_ optimization should now be taken literally when the
> optimization
> was disabled by the user via a (default) choice of a --param value
> for example
> (or a conflicting different optimization option).
> 
> The granularity for reporting -fopt-info is optimization groups (see
> dumpfile.h, the OPTGROUP_* enum), I believe
> maybe_complain_about_tail_call
> invocations are all from RTL expansion which currently is
> OPTGROUP_NONE which would have to change (I think it's currently
> not possible to explicitely specify an alternate optgroup at
> dump_printf time
> (David?).

IIRC, the dumping machinery tries to consolidate the dump messages into
"optinfo" instances, where a dump_*_loc call starts a new optinfo, and
it's then that the optgroup can be specified.  But it's been a while
since I last looked at this code.

> 
> So the simplest way forward would be to add, say, OPTGROUP_EXPAND,
> use dump_printf in maybe_complain_about_tail_call (which will also
> dump info to the .expand debug dump which looks useful) and recognize
> -fopt-info-expand.

[...snip...]

Dave

Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Bradley Lucier]

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

[Richard Biener]

On Wed, Aug 16, 2023 at 12:48 AM Bradley Lucier <lucier@purdue.edu> wrote:
>
> 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.

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.

Richard.

>
> Brad

Re: [PATCH] Add -Wdisabled-optimization warning for not optimizing sibling calls

[Bradley Lucier]

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

[Richard Biener]

On Fri, Aug 18, 2023 at 7:13 PM Bradley Lucier <lucier@purdue.edu> wrote:
>
> 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?

-fopt-info is for the user.  Note when you dump -all instead of
-missed or -optimized
you will get a lot of extra and eventually less useful things.  Basically
-fopt-info splices out info from passes that's both useful to users and also
things that are mostly interesting to compiler developers (that's -all).

> 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?

Yeah, that looks like a mis-classification to me.  It's using
the assigned CIF code (cif-code.def), and yes, we don't
have a function body for __builtin_expect but that's not the
reason we should present to users.  Instead of
MSG_MISSED_OPTIMIZATION for these class of callees
we should use MSG_NOTE.  At least to me a missed
optimization should point out places we would eventually
expect to be optimized either by providing enough of
the constraints required in the source or by fixing a missed
optimization capability in the compiler.

Richard.

> Brad