On Thu, 7 Sep 2017, Richard Biener wrote:
> 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)
After some pondering I decided for PHIs with all undefs there's
really no way things can go wrong.
Thus the following is what I installed.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Richard.
2017-09-14 Richard Biener <rguent...@suse.de>
* tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
to VN_TOP.
* gcc.dg/tree-ssa/ssa-fre-59.c: New testcase.
* gcc.dg/uninit-suppress_2.c: Adjust.
* gcc.dg/tree-ssa/ssa-sccvn-2.c: Likewise.
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c (revision 252062)
+++ gcc/tree-ssa-sccvn.c (working copy)
@@ -3860,11 +3860,11 @@ visit_reference_op_store (tree lhs, tree
static bool
visit_phi (gimple *phi)
{
- bool changed = false;
- tree result;
- tree sameval = VN_TOP;
- bool allsame = true;
+ tree result, sameval = VN_TOP, seen_undef = NULL_TREE;
unsigned n_executable = 0;
+ bool allsame = true;
+ edge_iterator ei;
+ edge e;
/* TODO: We could check for this in init_sccvn, and replace this
with a gcc_assert. */
@@ -3873,8 +3873,6 @@ visit_phi (gimple *phi)
/* See if all non-TOP arguments have the same value. TOP is
equivalent to everything, so we can ignore it. */
- edge_iterator ei;
- edge e;
FOR_EACH_EDGE (e, ei, gimple_bb (phi)->preds)
if (e->flags & EDGE_EXECUTABLE)
{
@@ -3884,8 +3882,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 for sameval but record one. */
+ 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,30 +3895,39 @@ visit_phi (gimple *phi)
break;
}
}
-
- /* 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. */
- if (sameval == VN_TOP
- || n_executable == 1)
- return set_ssa_val_to (PHI_RESULT (phi), sameval);
+
+ /* If none of the edges was executable keep the value-number at VN_TOP,
+ if only a single edge is exectuable use its value. */
+ if (n_executable <= 1)
+ result = seen_undef ? seen_undef : sameval;
+ /* If we saw only undefined values create a new undef SSA name to
+ avoid false equivalences. */
+ else if (sameval == VN_TOP)
+ {
+ gcc_assert (seen_undef);
+ result = seen_undef;
+ }
/* First see if it is equivalent to a phi node in this block. We prefer
this as it allows IV elimination - see PRs 66502 and 67167. */
- result = vn_phi_lookup (phi);
- if (result)
- changed = set_ssa_val_to (PHI_RESULT (phi), result);
- /* Otherwise all value numbered to the same value, the phi node has that
- value. */
- else if (allsame)
- changed = set_ssa_val_to (PHI_RESULT (phi), sameval);
+ else if ((result = vn_phi_lookup (phi)))
+ ;
+ /* If all values are the same use that, unless we've seen undefined
+ values as well and the value isn't constant.
+ CCP/copyprop have the same restriction to not remove uninit warnings. */
+ else if (allsame
+ && (! seen_undef || is_gimple_min_invariant (sameval)))
+ result = sameval;
else
{
- vn_phi_insert (phi, PHI_RESULT (phi));
- changed = set_ssa_val_to (PHI_RESULT (phi), PHI_RESULT (phi));
+ result = PHI_RESULT (phi);
+ /* Only insert PHIs that are varying, for constant value numbers
+ we mess up equivalences otherwise as we are only comparing
+ the immediate controlling predicates. */
+ vn_phi_insert (phi, result);
}
- return changed;
+ return set_ssa_val_to (PHI_RESULT (phi), result);
}
/* Try to simplify RHS using equivalences and constant folding. */
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/testsuite/gcc.dg/uninit-suppress_2.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-suppress_2.c (revision 252062)
+++ gcc/testsuite/gcc.dg/uninit-suppress_2.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-fno-tree-ccp -fno-tree-vrp -O2 -Wuninitialized
-Werror=uninitialized -Wno-error=maybe-uninitialized" } */
+/* { dg-options "-fno-tree-ccp -fno-tree-vrp -fno-tree-fre -fno-tree-pre
-fno-code-hoisting -O2 -Wuninitialized -Werror=uninitialized
-Wno-error=maybe-uninitialized" } */
void blah();
void bar (int);
int gflag;
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-2.c (revision 252062)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-2.c (working copy)
@@ -21,4 +21,4 @@ int vnum_test8(int *data)
}
/* We should eliminate m - n, and set n = n + k into n = m, and
set p to 0 */
-/* { dg-final { scan-tree-dump-times "Eliminated: 4" 1 "fre1"} } */
+/* { dg-final { scan-tree-dump-times "Eliminated: 5" 1 "fre1"} } */
