On Thu, 7 Sep 2017, Richard Biener wrote:
>
> This enhances VN to do the same PHI handling as CCP, meeting
> undefined and constant to constant. I've gone a little bit
> further (and maybe will revisit this again) in also meeting
> all-undefined to undefined taking one of the undefined args
> as the value number. I feel like this might run into
> the equation issues I mentioned in the other mail so it
> would be cleaner to invent a "new" undefined value number
> here -- but I have to make sure to not create too many
> default-defs or break iteration convergence (default defs are also
> expensive given they require a decl - sth I want to change as well).
>
> So for now I guess I'll stick with the slightly bogus(?) way also
> hoping for a testcase that shows the issue this uncovers.
>
> Note it's required to handle
>
> _3 = PHI <_1(D), _2(D)>
> ..
>
> _4 = PHI <_3, 1>
>
> consistently with
>
> _4 = PHI <_1(D), _2(D), 1>
>
> aka with/without extra forwarders.
That said, "fallout" is we simplify
int foo (int b)
{
int i, j, k;
if (b)
k = i;
else
k = j;
if (k == i)
return 1;
else if (k == j)
return 2;
return 0;
to
if (j == i)
return 1;
else
return 2;
or even just
return 2;
dependent on PHI argument order of k = PHI <i(D), j(D)>.
Likewise we'd say that either k - i or k - j is zero.
The complication with PHIs is that they do not always only appear
in places where uses of the args dominate it but the other way
around so we can't really invoke the undefined behavior rule
on a PHI node with undefined args itself. The question is whether
we may for PHIs with just undefined args ... but my guess is no
so I do have to fix the above.
Anybody can produce a testcase that we'd consider wrong-code?
(the above examples clearly are not)
Thanks,
Richard.
> Similar to CCP/copyprop this doesn't allow copies to be propagated
> this way as this removes most gcc.dg/Wuninitialized* warnings
> we test for...
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Richard.
>
> 2017-09-07 Richard Biener <rguent...@suse.de>
>
> * tree-ssa-sccnv.c (visit_phi): Handle meeting undefined
> values.
>
> * gcc.dg/tree-ssa/ssa-fre-59.c: New testcase.
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c (working copy)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-fre1" } */
> +
> +int i;
> +int foo (int b)
> +{
> + int j;
> + i = 1;
> + if (b)
> + j = i;
> + return i - j;
> +}
> +
> +/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> --- gcc/tree-ssa-sccvn.c (revision 251833)
> +++ gcc/tree-ssa-sccvn.c (working copy)
> @@ -3864,6 +3864,7 @@ visit_phi (gimple *phi)
> tree result;
> tree sameval = VN_TOP;
> bool allsame = true;
> + tree seen_undef = NULL_TREE;
> unsigned n_executable = 0;
>
> /* TODO: We could check for this in init_sccvn, and replace this
> @@ -3884,8 +3885,12 @@ visit_phi (gimple *phi)
> if (TREE_CODE (def) == SSA_NAME)
> def = SSA_VAL (def);
> if (def == VN_TOP)
> - continue;
> - if (sameval == VN_TOP)
> + ;
> + /* Ignore undefined defs here. */
> + else if (TREE_CODE (def) == SSA_NAME
> + && ssa_undefined_value_p (def, false))
> + seen_undef = def;
> + else if (sameval == VN_TOP)
> sameval = def;
> else if (!expressions_equal_p (def, sameval))
> {
> @@ -3893,7 +3898,18 @@ visit_phi (gimple *phi)
> break;
> }
> }
> -
> + /* If we found undefined values and didn't end up with a constant
> + drop back to varying as otherwise we end up removing most late
> + uninitialized use warnings. CCP/copyprop have the same restriction. */
> + if (allsame && seen_undef)
> + {
> + /* Unless this was the only value we've seen. */
> + if (sameval == VN_TOP)
> + sameval = seen_undef;
> + else if (! is_gimple_min_invariant (sameval))
> + allsame = false;
> + }
> +
> /* If none of the edges was executable or all incoming values are
> undefined keep the value-number at VN_TOP. If only a single edge
> is exectuable use its value. */
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
