On Mon, 16 Nov 2020, Jan Hubicka wrote:
> > On Nov 15 2020, Jan Hubicka wrote:
> >
> > >> See PR97840.
> > > Thanks,
> > > this is a false positive where we fail to discover that pointed-to type
> > > is empty on non-x86_64 targets. This is triggered by better alias
> > > analysis caused by non-escape discovery.
> > >
> > > While this is not a full fix (I hope someone with more experience on
> > > C++ types and warnings will set up) I think it may be useful to avoid
> > > warning on unused parameter.
> > >
> > > Bootstrapped/regtested x86_64-linux, OK?
> >
> > That doesn't fix anything.
>
> It does silence the warning if you remove inline from function
> declaration (as creduce while minimizing the testcase - the minimized
> testcase was undefined that is why I did not include it at the end).
> I now implemented one by hand.
>
> The reason is that gimple_call_arg_flags clears EAF_UNUSED
> on symbols that !binds_to_current_def_p because we are worried that
> symbol will be interposed by different version of the body with
> same semantics that will actually read the arg.
> This is bit paranoid check since we optimize things like *a == *a early
> but with clang *a will trap if a==NULL.
>
> Richi, I think we can add "safe" parameter to gimple_call_arg_flags and
> bypass this logic when we use it for warnings only (having body that
> does not use the value is quite strong hint that it is unused by the
> function).
Eh, please not.
>
> I played with bit more testcases and found that we also want to disable
> warning for const functions and sometimes EAF_UNUSED flag is dropped
> becaue of clobber that is not necessary to do. If function only clobber
> the target it can be considered unused past inlining.
>
> I am testing this improved patch and plan to commit if there are no
> complains, but still we need to handle binds_to_current_def.
>
> On the other direction, Martin, I think we may also warn for args
> that are !EAF_UNUSED and !EAF_NOCLOBBER. This will catch some cases
> where user did not add "const" specifier to the declaration but
> parameter is detected to be readonly.
>
> I also noticed that we do not detect EAF_UNUSED for fully unused
> parameters (with no SSA names associated with them). I will fix that
> incrementally.
Make sure to not apply it based on that reason to aggregates ;)
> Honza
>
> PR middle-end/97840
> * ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining
> is done.
> * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall;
> skip const calls and unused arguments.
> (warn_uninitialized_vars): Update prototype.
>
> gcc/testsuite/ChangeLog:
>
> 2020-11-16 Jan Hubicka <hubi...@ucw.cz>
>
> * g++.dg/warn/unit-2.C: New test.
>
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 4a43c50aa66..08fcc36a321 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec<unsigned char>
> &known_flags, int depth)
> /* Handle *name = exp. */
> else if (assign
> && memory_access_to (gimple_assign_lhs (assign), name))
> - flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> + {
> + /* In general we can not ignore clobbers because they are
> + barriers for code motion, however after inlining it is safe to
> + do because local optimization passes do not consider clobbers
> + from other functions. Similar logic is in ipa-pure-const.c.
> */
> + if (!cfun->after_inlining && !gimple_clobber_p (assign))
> + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
> + }
> /* ASM statements etc. */
> else if (!assign)
> {
> diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C
> b/gcc/testsuite/g++.dg/warn/unit-2.C
> new file mode 100644
> index 00000000000..30f3ceae191
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/unit-2.C
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wmaybe-uninitialized" } */
> +struct a {int a;};
> +__attribute__ ((noinline))
> +void
> +nowarn (const struct a *ptr)
> +{
> + if (ptr)
> + asm volatile ("");
> +}
> +void
> +test()
> +{
> + struct a ptr;
> + nowarn (&ptr);
> +}
> +__attribute__ ((noinline))
> +int
> +nowarn2 (const struct a *ptr, const struct a ptr2)
> +{
> + return ptr != 0 || ptr2.a;
> +}
> +int mem;
> +int
> +test2()
> +{
> + struct a ptr,ptr2={0};
> + return nowarn2 (&ptr, ptr2);
> +}
> diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
> index f23514395e0..c94831bfb75 100644
> --- a/gcc/tree-ssa-uninit.c
> +++ b/gcc/tree-ssa-uninit.c
> @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs,
> tree rhs,
> access implying read access to those objects. */
>
> static void
> -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
> +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
> {
> if (!wlims.wmaybe_uninit)
> return;
> @@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits
> &wlims)
> if (!fntype)
> return;
>
> + /* Const function do not read their arguments. */
> + if (gimple_call_flags (stmt) & ECF_CONST)
> + return;
> +
> const built_in_function fncode
> = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
> ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST);
> @@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits
> &wlims)
> (but not definitive) read access. */
> wlims.always_executed = false;
>
> + /* Ignore args we are not going to read from. */
> + if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
> + continue;
> +
> tree arg = gimple_call_arg (stmt, argno - 1);
>
> ao_ref ref;
> @@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
> if (gimple_vdef (stmt))
> wlims.vdef_cnt++;
>
> - if (is_gimple_call (stmt))
> - maybe_warn_pass_by_reference (stmt, wlims);
> + if (gcall *call = dyn_cast <gcall *> (stmt))
> + maybe_warn_pass_by_reference (call, wlims);
> else if (gimple_assign_load_p (stmt)
> && gimple_has_location (stmt))
> {
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
