On Mon, 3 Sep 2018, Richard Biener wrote:
>
> The following fixes a wrong-code issue similar to that in PR87132 where
> the alias walk reaches into code parts that are only reachable over
> backedges. This time not via PHI processing but by value-numbering
> a virtual PHI to its backedge value. That doesn't play well with the
> way we do iteration for the memory state.
>
> This also adds a testcase showing the desirable way of doing virtual
> PHI value-numbering (ideally the walking code would honor edge
> executability state but that would tie it even more to VN...).
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
This is what I have applied.
Richard.
2018-09-04 Richard Biener <rguent...@suse.de>
PR tree-optimization/87176
* tree-ssa-sccvn.c (visit_phi): Remove redundant allsame
variable. When value-numbering a virtual PHI node make sure
to not value-number to the backedge value.
* gcc.dg/torture/pr87176.c: New testcase.
* gcc.dg/torture/ssa-fre-1.c: Likewise.
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c (revision 264069)
+++ gcc/tree-ssa-sccvn.c (working copy)
@@ -4110,11 +4110,11 @@ static bool
visit_phi (gimple *phi, bool *inserted, bool backedges_varying_p)
{
tree result, sameval = VN_TOP, seen_undef = NULL_TREE;
- tree backedge_name = NULL_TREE;
+ tree backedge_val = NULL_TREE;
+ bool seen_non_backedge = false;
tree sameval_base = NULL_TREE;
poly_int64 soff, doff;
unsigned n_executable = 0;
- bool allsame = true;
edge_iterator ei;
edge e;
@@ -4139,11 +4139,13 @@ visit_phi (gimple *phi, bool *inserted,
++n_executable;
if (TREE_CODE (def) == SSA_NAME)
{
- if (e->flags & EDGE_DFS_BACK)
- backedge_name = def;
if (!backedges_varying_p || !(e->flags & EDGE_DFS_BACK))
def = SSA_VAL (def);
+ if (e->flags & EDGE_DFS_BACK)
+ backedge_val = def;
}
+ if (!(e->flags & EDGE_DFS_BACK))
+ seen_non_backedge = true;
if (def == VN_TOP)
;
/* Ignore undefined defs for sameval but record one. */
@@ -4172,16 +4174,25 @@ visit_phi (gimple *phi, bool *inserted,
&& known_eq (soff, doff))
continue;
}
- allsame = false;
+ sameval = NULL_TREE;
break;
}
}
/* If the value we want to use is the backedge and that wasn't visited
- yet drop to VARYING. */
- if (backedge_name
- && sameval == backedge_name
- && !SSA_VISITED (backedge_name))
+ yet drop to VARYING. This only happens when not iterating.
+ If we value-number a virtual operand never value-number to the
+ value from the backedge as that confuses the alias-walking code.
+ See gcc.dg/torture/pr87176.c. If the value is the same on a
+ non-backedge everything is OK though. */
+ if (backedge_val
+ && !seen_non_backedge
+ && TREE_CODE (backedge_val) == SSA_NAME
+ && sameval == backedge_val
+ && (SSA_NAME_IS_VIRTUAL_OPERAND (backedge_val)
+ || !SSA_VISITED (backedge_val)))
+ /* Note this just drops to VARYING without inserting the PHI into
+ the hashes. */
result = PHI_RESULT (phi);
/* If none of the edges was executable keep the value-number at VN_TOP,
if only a single edge is exectuable use its value. */
@@ -4212,7 +4223,7 @@ visit_phi (gimple *phi, bool *inserted,
/* 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
+ else if (sameval
&& (! seen_undef || is_gimple_min_invariant (sameval)))
result = sameval;
else
Index: gcc/testsuite/gcc.dg/torture/pr87176.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr87176.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr87176.c (working copy)
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+
+int a, b, c;
+
+int main ()
+{
+ int d = a = 0;
+ while (1)
+ {
+ a = a ^ 6;
+ if (!a)
+ break;
+ if (d)
+ goto L;
+ d = a;
+ for (b = 0; b < 2; b++)
+ {
+ const int *f[3] = { &c };
+ const int **g[] = { &f[2] };
+ int h = ~d;
+ if (d)
+ L:
+ if (h > 1)
+ continue;
+ }
+ }
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/ssa-fre-1.c (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-1.c (working copy)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fstrict-aliasing -fdump-tree-fre1" } */
+
+float f;
+int foo(int *p, int *q)
+{
+ *p = 0;
+ if (*p)
+ *q = 1;
+ else
+ f = 8.0f;
+ return *p;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
