On Mon, 20 Feb 2017, Jakub Jelinek wrote:
> Hi!
>
> As mentioned by Jason in the PR, we've regressed on the following testcase
> since we started emitting CLOBBERs at the start of ctors (and we warn as
> before with -fno-lifetime-dse -Wuninitialized).
> With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
> and thus we warn, but if there are clobbers before that, that is not the
> case and we don't warn. The patch is quick hack to bypass the initial
> clobbers as long as there aren't really many. If we wanted to handle
> all initial clobbers, I bet the first time we run into this we could
> recursively walk vop uses from the default def and build say a bitmap
> of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
> before it as vdef stmts.
>
> Now, the comment says:
> /* For memory the only cheap thing we can do is see if we
> have a use of the default def of the virtual operand.
> ??? Not so cheap would be to use the alias oracle via
> walk_aliased_vdefs, if we don't find any aliasing vdef
> warn as is-used-uninitialized, if we don't find an aliasing
> vdef that kills our use (stmt_kills_ref_p), warn as
> may-be-used-uninitialized. But this walk is quadratic and
> so must be limited which means we would miss warning
> opportunities. */
> I wonder if it isn't useful to walk even limited number of vdefs this way
> anyway (likely GCC8 material though), e.g. if we run into a clobber that
> must (rather than just may) portion of the read ref (and of course when
> seeing non-clobber stmt that could alias with the ref give up before that),
> we could warn even if we are very far from the start of the function.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (or do
> you want a version with a bitmap and really ignoring all the clobbers,
> rather than just 128 of them)?
I'd rather not special-case clobbers but take this as an opportunity
to implement that ??? comment with walk_aliased_vdefs (you can do
some copy&paste programming from the use in tree-ssa-dce.c I think).
Just use a very low walking limit.
As you say, a kill from a clobber should warn (you skip them).
Richard.
> 2017-02-20 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/79345
> * tree-ssa-uninit.c (warn_uninitialized_vars): If vuse is not
> default def, but there are only CLOBBER stmts as vdefs, handle it like
> default def.
>
> * g++.dg/warn/pr79345.C: New test.
>
> --- gcc/tree-ssa-uninit.c.jj 2017-01-01 12:45:35.000000000 +0100
> +++ gcc/tree-ssa-uninit.c 2017-02-20 16:43:08.244868075 +0100
> @@ -248,8 +248,7 @@ warn_uninitialized_vars (bool warn_possi
> use = gimple_vuse (stmt);
> if (use
> && gimple_assign_single_p (stmt)
> - && !gimple_vdef (stmt)
> - && SSA_NAME_IS_DEFAULT_DEF (use))
> + && !gimple_vdef (stmt))
> {
> tree rhs = gimple_assign_rhs1 (stmt);
> tree base = get_base_address (rhs);
> @@ -260,6 +259,23 @@ warn_uninitialized_vars (bool warn_possi
> || is_global_var (base))
> continue;
>
> + /* Look through some CLOBBER stmts. */
> + for (unsigned int cnt = 0; cnt < 128 && use; cnt++)
> + {
> + if (SSA_NAME_IS_DEFAULT_DEF (use))
> + break;
> +
> + gimple *def_stmt = SSA_NAME_DEF_STMT (use);
> + if (!gimple_clobber_p (def_stmt))
> + {
> + use = NULL_TREE;
> + break;
> + }
> + use = gimple_vuse (def_stmt);
> + }
> + if (use == NULL_TREE)
> + continue;
> +
> if (always_executed)
> warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1 (stmt),
> base, "%qE is used uninitialized in this function",
> --- gcc/testsuite/g++.dg/warn/pr79345.C.jj 2017-02-20 17:19:01.138952915
> +0100
> +++ gcc/testsuite/g++.dg/warn/pr79345.C 2017-02-20 17:18:20.000000000
> +0100
> @@ -0,0 +1,22 @@
> +// PR tree-optimization/79345
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -Wuninitialized" }
> +
> +struct A {
> + A (int);
> +};
> +
> +struct B : A {
> + const bool x = true;
> +
> + B () : A (x ? 3 : 7) { } // { dg-warning "is used uninitialized in this
> function" }
> +};
> +
> +void foo (void*);
> +
> +void
> +bar ()
> +{
> + B b;
> + foo (&b);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
