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