[PATCH] tree-optimization/93354 FRE redundant store removal validity fix

[Richard Biener <rguenther at suse dot de>]

This fixes the validity check for redundant store removal by looking
at the actual stmt that represents the last (relevant) change to
the TBAA state rather than the imperfectly tracked alias set in the
expression hash table which then becomes unused (see a followup change
to get rid of that).

On the way this allows re-enabling of VN_WALKREWRITE, fixing PR91123.

This also exposes an out-of-bound array access in 
real.c:clear_significand_below fixed as well.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2020-01-22  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91123
	PR tree-optimization/93381
	* tree-ssa-sccvn.c (store_affects_tbaa_memory_state): New function.
	(visit_reference_op_store): Use it to check validity of eliding
	redundant stores.
	(eliminate_dom_walker::eliminate_stmt): Likewise, allow lookup
	to look through aggregate copies.
	(vn_reference_lookup): Consistently set *last_use_ptr.
	* real.c (clear_significand_below): Fix out-of-bound access.

	* gcc.dg/torture/pr93354.c: New testcase.
	* gcc.dg/tree-ssa/ssa-fre-85.c: Likewise.

diff --git a/gcc/real.c b/gcc/real.c
index a57e5df113f..46d8126efe4 100644
--- a/gcc/real.c
+++ b/gcc/real.c
@@ -420,7 +420,10 @@ clear_significand_below (REAL_VALUE_TYPE *r, unsigned int n)
   for (i = 0; i < w; ++i)
     r->sig[i] = 0;
 
-  r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
+  /* We are actually passing N == SIGNIFICAND_BITS which would result
+     in an out-of-bound access below.  */
+  if (n % HOST_BITS_PER_LONG != 0)
+    r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
 }
 
 /* Divide the significands of A and B, placing the result in R.  Return
diff --git a/gcc/testsuite/gcc.dg/torture/pr93354.c b/gcc/testsuite/gcc.dg/torture/pr93354.c
new file mode 100644
index 00000000000..8ccf1e6d2c0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93354.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+
+typedef __INT32_TYPE__ int32_t;
+typedef __INT64_TYPE__ int64_t;
+struct X { int32_t i; int32_t j; };
+void foo (int64_t *z)
+{
+  ((struct X *)z)->i = 0x05060708;
+  ((struct X *)z)->j = 0x01020304;
+  *z = 0x0102030405060708;
+}
+
+int main()
+{
+  int64_t l = 0;
+  int64_t *p;
+  asm ("" : "=r" (p) : "0" (&l));
+  foo (p);
+  if (l != 0x0102030405060708)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
new file mode 100644
index 00000000000..c50770caa2f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fstrict-aliasing -fdump-tree-fre1-details" } */
+
+struct X { int i; int j; };
+
+struct X x, y;
+void foo ()
+{
+  x.i = 1;
+  y = x;
+  y.i = 1; // redundant
+}
+
+/* { dg-final { scan-tree-dump "Deleted redundant store y.i" "fre1" } } */
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 0b8ee586139..ef272047bb3 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -3205,6 +3205,8 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
       return NULL_TREE;
     }
 
+  if (last_vuse_ptr)
+    *last_vuse_ptr = vr1.vuse;
   return vn_reference_lookup_1 (&vr1, vnresult);
 }
 
@@ -4543,6 +4545,23 @@ visit_reference_op_load (tree lhs, tree op, gimple *stmt)
 }
 
 
+/* Returns whether the store to LHS affects dependence analysis compared
+   to the next preceeding stmt doing so at VUSE.  */
+
+static bool
+store_affects_tbaa_memory_state (tree lhs, tree vuse)
+{
+  gassign *def = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (vuse));
+  if (!def)
+    return true;
+  alias_set_type lhs_set = get_alias_set (lhs);
+  alias_set_type def_set = get_alias_set (gimple_assign_lhs (def));
+  if (lhs_set != def_set
+      && !alias_set_subset_of (lhs_set, def_set))
+    return true;
+  return false;
+}
+
 /* Visit a store to a reference operator LHS, part of STMT, value number it,
    and return true if the value number of the LHS has changed as a result.  */
 
@@ -4575,7 +4594,8 @@ visit_reference_op_store (tree lhs, tree op, gimple *stmt)
      Otherwise, the vdefs for the store are used when inserting into
      the table, since the store generates a new memory state.  */
 
-  vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false);
+  tree last_vuse = NULL_TREE;
+  vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false, &last_vuse);
   if (vnresult
       && vnresult->result)
     {
@@ -4587,9 +4607,7 @@ visit_reference_op_store (tree lhs, tree op, gimple *stmt)
 	{
 	  /* If the TBAA state isn't compatible for downstream reads
 	     we cannot value-number the VDEFs the same.  */
-	  alias_set_type set = get_alias_set (lhs);
-	  if (vnresult->set != set
-	      && ! alias_set_subset_of (set, vnresult->set))
+	  if (store_affects_tbaa_memory_state (lhs, last_vuse))
 	    resultsame = false;
 	}
     }
@@ -5644,9 +5662,11 @@ eliminate_dom_walker::eliminate_stmt (basic_block b, gimple_stmt_iterator *gsi)
 	    lookup_lhs = NULL_TREE;
 	}
       tree val = NULL_TREE;
+      tree last_vuse = NULL_TREE;
       if (lookup_lhs)
-	val = vn_reference_lookup (lookup_lhs, gimple_vuse (stmt), VN_WALK,
-				   &vnresult, false);
+	val = vn_reference_lookup (lookup_lhs, gimple_vuse (stmt),
+				   VN_WALKREWRITE, &vnresult, false,
+				   &last_vuse);
       if (TREE_CODE (rhs) == SSA_NAME)
 	rhs = VN_INFO (rhs)->valnum;
       if (val
@@ -5660,13 +5680,12 @@ eliminate_dom_walker::eliminate_stmt (basic_block b, gimple_stmt_iterator *gsi)
 		  && TREE_CODE (val) == VIEW_CONVERT_EXPR
 		  && TREE_OPERAND (val, 0) == rhs)))
 	{
-	  /* We can only remove the later store if the former aliases
-	     at least all accesses the later one does or if the store
+	  /* We now proved the memory contents before and after the store
+	     are the same.  We can only remove the later store if the former
+	     aliases at least all accesses the later one does or if the store
 	     was to readonly memory storing the same value.  */
-	  alias_set_type set = get_alias_set (lhs);
 	  if (! vnresult
-	      || vnresult->set == set
-	      || alias_set_subset_of (set, vnresult->set))
+	      || !store_affects_tbaa_memory_state (lhs, last_vuse))
 	    {
 	      if (dump_file && (dump_flags & TDF_DETAILS))
 		{