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