This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFC: Fix GCSE exp_equiv_p on MEMs with different MEM_ATTRS (PR rtl-optimization/49390)


Hi!

As the testcase shows, the
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg02945.html
patch looks wrong, MEM_ATTRS matters quite a lot for the
alias oracle, so ignoring it leads to miscompilations.

Instead of just reverting the patch, this patch attempts to add
some exceptions, notably when MEM_ATTRS are indirect with MEM_REF
containing an SSA_NAME as base, and the SSA_NAMEs have the same
var (maybe that check is unnecessary) and both SSA_NAMEs have the
same points-to info, we can consider them interchangeable.
But when they have different points-to info or if other details
are different, we need to play safe and return false from exp_equiv_p.

Bootstrapped/regtested on x86_64-linux and i686-linux.

Should the tests in rtx_mem_attrs_equiv_p be split into more
functions (e.g. have a points-to equiv inline function, and
perhaps most of the remaining body in tree-ssa-alias.c instead
of alias.c (like just do the ao_ref_from_mem/MEM_ATTRS/MEM_ALIGN
checks in there and keep the rest in the tree files)?

2011-06-13  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/49390
	* alias.c (rtx_mem_attrs_equiv_p): New function.
	* alias.h (rtx_mem_attrs_equiv_p): New prototype.
	* tree-ssa-alias.c (ao_ref_base_alias_set): No longer static.
	* tree-ssa-alias.h (ao_ref_base_alias_set): New prototype.
	* cse.c (exp_equiv_p) <case MEM>: If MEM_ATTRS are different,
	call rtx_mem_attrs_equiv_p to see if MEM_ATTRS are interchangeable.

	* gcc.c-torture/execute/pr49390.c: New test.

--- gcc/alias.c.jj	2011-05-24 23:34:28.000000000 +0200
+++ gcc/alias.c	2011-06-13 17:17:17.000000000 +0200
@@ -374,6 +374,79 @@ rtx_refs_may_alias_p (const_rtx x, const
 			     && MEM_ALIAS_SET (mem) != 0);
 }
 
+/* Return true if MEM_ATTRS of X and Y are equivalent for the
+   alias oracle.  */
+
+bool
+rtx_mem_attrs_equiv_p (const_rtx x, const_rtx y)
+{
+  ao_ref ref1, ref2;
+
+  if (MEM_ATTRS (x) == MEM_ATTRS (y))
+    return true;
+
+  if (!ao_ref_from_mem (&ref1, x)
+      || !ao_ref_from_mem (&ref2, y))
+    return false;
+
+  if (MEM_ALIGN (x) != MEM_ALIGN (y))
+    return false;
+
+  if (ref1.base == NULL
+      || ref2.base == NULL
+      || ref1.ref == NULL
+      || ref2.ref == NULL
+      || MEM_ALIGN (x) != MEM_ALIGN (y)
+      || TREE_CODE (ref1.ref) != TREE_CODE (ref2.ref)
+      || TREE_CODE (ref1.base) != TREE_CODE (ref2.base)
+      || ref1.offset != ref2.offset
+      || ref1.size != ref2.size
+      || ref1.max_size != ref2.max_size
+      || ao_ref_alias_set (&ref1) != ao_ref_alias_set (&ref2)
+      || ao_ref_base_alias_set (&ref1) != ao_ref_base_alias_set (&ref2))
+    return false;
+
+  if (TREE_CODE (ref1.base) == MEM_REF
+      && TREE_CODE (TREE_OPERAND (ref1.base, 0)) == SSA_NAME
+      && TREE_CODE (TREE_OPERAND (ref2.base, 0)) == SSA_NAME)
+    {
+      tree v1 = TREE_OPERAND (ref1.base, 0);
+      tree v2 = TREE_OPERAND (ref2.base, 0);
+      struct ptr_info_def *pi1, *pi2;
+
+      if (SSA_NAME_VAR (v1) != SSA_NAME_VAR (v2)
+	  || !types_compatible_p (TREE_TYPE (ref1.base), TREE_TYPE (ref2.base))
+	  || TREE_OPERAND (ref1.base, 1) != TREE_OPERAND (ref2.base, 1))
+	return false;
+
+      pi1 = SSA_NAME_PTR_INFO (v1);
+      pi2 = SSA_NAME_PTR_INFO (v2);
+      if (pi1 == NULL || pi2 == NULL)
+	return pi1 == NULL && pi2 == NULL;
+      if (pi1->align != pi2->align
+	  || pi1->misalign != pi2->misalign
+	  || pi1->pt.anything != pi2->pt.anything
+	  || pi1->pt.nonlocal != pi2->pt.nonlocal
+	  || pi1->pt.escaped != pi2->pt.escaped
+	  || pi1->pt.ipa_escaped != pi2->pt.ipa_escaped
+	  || pi1->pt.null != pi2->pt.null
+	  || pi1->pt.vars_contains_global != pi2->pt.vars_contains_global
+	  || pi1->pt.vars_contains_restrict != pi2->pt.vars_contains_restrict)
+	return false;
+
+      if (pi1->pt.vars == NULL || pi2->pt.vars == NULL)
+	return pi1->pt.vars == NULL && pi2->pt.vars == NULL;
+
+      if (pi1->pt.vars == pi2->pt.vars
+	  || bitmap_equal_p (pi1->pt.vars, pi2->pt.vars))
+	return true;
+
+      return false;
+    }
+
+  return false;
+}
+
 /* Returns a pointer to the alias set entry for ALIAS_SET, if there is
    such an entry, or NULL otherwise.  */
 
--- gcc/alias.h.jj	2011-01-06 10:21:52.000000000 +0100
+++ gcc/alias.h	2011-06-13 17:05:46.000000000 +0200
@@ -43,6 +43,7 @@ extern int alias_sets_conflict_p (alias_
 extern int alias_sets_must_conflict_p (alias_set_type, alias_set_type);
 extern int objects_must_conflict_p (tree, tree);
 extern int nonoverlapping_memrefs_p (const_rtx, const_rtx, bool);
+extern bool rtx_mem_attrs_equiv_p (const_rtx, const_rtx);
 
 /* This alias set can be used to force a memory to conflict with all
    other memories, creating a barrier across which no memory reference
--- gcc/tree-ssa-alias.c.jj	2011-05-31 08:03:10.000000000 +0200
+++ gcc/tree-ssa-alias.c	2011-06-13 17:16:36.000000000 +0200
@@ -489,7 +489,7 @@ ao_ref_base (ao_ref *ref)
 
 /* Returns the base object alias set of the memory reference *REF.  */
 
-static alias_set_type
+alias_set_type
 ao_ref_base_alias_set (ao_ref *ref)
 {
   tree base_ref;
--- gcc/tree-ssa-alias.h.jj	2011-05-02 18:39:28.000000000 +0200
+++ gcc/tree-ssa-alias.h	2011-05-02 18:39:28.000000000 +0200
@@ -96,6 +96,7 @@ extern void ao_ref_init (ao_ref *, tree)
 extern void ao_ref_init_from_ptr_and_size (ao_ref *, tree, tree);
 extern tree ao_ref_base (ao_ref *);
 extern alias_set_type ao_ref_alias_set (ao_ref *);
+extern alias_set_type ao_ref_base_alias_set (ao_ref *);
 extern bool ptr_deref_may_alias_global_p (tree);
 extern bool ptr_derefs_may_alias_p (tree, tree);
 extern bool refs_may_alias_p (tree, tree);
--- gcc/cse.c.jj	2011-06-06 10:24:41.000000000 +0200
+++ gcc/cse.c	2011-06-13 17:11:18.000000000 +0200
@@ -2679,6 +2679,19 @@ exp_equiv_p (const_rtx x, const_rtx y, i
 	     other.  */
 	  if (MEM_VOLATILE_P (x) || MEM_VOLATILE_P (y))
 	    return 0;
+
+	  /* First check the address before testing MEM_ATTRS.  */
+	  if (!exp_equiv_p (XEXP (x, 0), XEXP (y, 0), validate, for_gcse))
+	    return 0;
+
+	  /* If MEM_ATTRS are different, generally we can't merge
+	     the MEMs, as alias oracle could behave differently for them.
+	     There are a few exceptions where we can detect it will behave
+	     the same though.  */
+	  if (MEM_ATTRS (x) != MEM_ATTRS (y) && !rtx_mem_attrs_equiv_p (x, y))
+	    return 0;
+
+	  return 1;
 	}
       break;
 
--- gcc/testsuite/gcc.c-torture/execute/pr49390.c.jj	2011-06-13 17:28:09.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr49390.c	2011-06-13 17:27:49.000000000 +0200
@@ -0,0 +1,88 @@
+/* PR rtl-optimization/49390 */
+
+struct S { unsigned int s1; unsigned int s2; };
+struct T { unsigned int t1; struct S t2; };
+struct U { unsigned short u1; unsigned short u2; };
+struct V { struct U v1; struct T v2; };
+struct S a;
+char *b;
+union { char b[64]; struct V v; } u;
+volatile int v;
+extern void abort (void);
+
+__attribute__((noinline, noclone)) void
+foo (int x, void *y, unsigned int z, unsigned int w)
+{
+  if (x != 4 || y != (void *) &u.v.v2)
+    abort ();
+  v = z + w;
+  v = 16384;
+}
+
+__attribute__((noinline, noclone)) void
+bar (struct S x)
+{
+  v = x.s1;
+  v = x.s2;
+}
+
+__attribute__((noinline, noclone)) int
+baz (struct S *x)
+{
+  v = x->s1;
+  v = x->s2;
+  v = 0;
+  return v + 1;
+}
+
+__attribute__((noinline, noclone)) void
+test (struct S *c)
+{
+  struct T *d;
+  struct S e = a;
+  unsigned int f, g;
+  if (c == 0)
+    c = &e;
+  else
+    {
+      if (c->s2 % 8192 <= 15 || (8192 - c->s2 % 8192) <= 31)
+	foo (1, 0, c->s1, c->s2);
+    }
+  if (!baz (c))
+    return;
+  g = (((struct U *) b)->u2 & 2) ? 32 : __builtin_offsetof (struct V, v2);
+  f = c->s2 % 8192;
+  if (f == 0)
+    {
+      e.s2 += g;
+      f = g;
+    }
+  else if (f < g)
+    {
+      foo (2, 0, c->s1, c->s2);
+      return;
+    }
+  if ((((struct U *) b)->u2 & 1) && f == g)
+    {
+      bar (*c);
+      foo (3, 0, c->s1, c->s2);
+      return;
+    }
+  d = (struct T *) (b + c->s2 % 8192);
+  if (d->t2.s1 >= c->s1 && (d->t2.s1 != c->s1 || d->t2.s2 >= c->s2))
+    foo (4, d, c->s1, c->s2);
+  return;
+}
+
+int
+main ()
+{
+  struct S *c = 0;
+  asm ("" : "+r" (c) : "r" (&a));
+  u.v.v2.t2.s1 = 8192;
+  b = u.b;
+  test (c);
+  if (v != 16384)
+    abort ();
+  return 0;
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]