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]

Re: [PATCH] Fix the one entry mem{{,p}cpy,move,set} optimization aliasing issues (PR middle-end/29272)


On Fri, Sep 29, 2006 at 10:42:02AM -0700, Mark Mitchell wrote:
> >2006-09-29  Jakub Jelinek  <jakub@redhat.com>
> >
> >	PR middle-end/29272
> >	* builtins.c (fold_builtin_memset, fold_builtin_memory_op): Restrict
> >	single entry optimization to variables and components thereof.
> 
> I think this idea is reasonable.  Unfortunately, I'd expect one of the 
> cases where this optimization to be most useful to be copying into 
> heap-allocated memory, but correctness trumps optimization...

Well, I believe (and pointed that even in bugzilla) that this optimization
is most useful when it actually makes a variable made addressable just
because of the memcpy/memset/etc. call no longer addressable.

> Would you please make sure that you have a conforming test case to check 
> in with this change, and that you add it to the PR?  The current test 
> cases in the PR aren't conforming, so as it stands the PR looks like a 
> non-bug; we should put in a test case which actually is miscompiled so 
> that it's obvious that the PR really is a bug.

Here is an updated patch even with the testcase.  As long as &p->t is not
considered an access to *p nor p->t (from Joseph's 
http://gcc.gnu.org/ml/gcc/2004-11/msg00978.html
mail I gather that it is unclear in the standard, but both common sense
and that mail suggests that &p->t is more likely not considered an access
to *p nor p->t), I believe the testcase is valid.

Bootstrapped/regtested on 7 linux arches, ok to commit in this form?

2006-10-04  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/29272
	* builtins.c (fold_builtin_memset, fold_builtin_memory_op): Restrict
	single entry optimization to variables and components thereof.

	* gcc.c-torture/execute/20060930-1.c: New test.

--- gcc/builtins.c.jj	2006-09-22 10:29:55.000000000 +0200
+++ gcc/builtins.c	2006-09-28 20:31:30.000000000 +0200
@@ -7905,7 +7905,7 @@ fold_builtin_exponent (tree fndecl, tree
 static tree
 fold_builtin_memset (tree arglist, tree type, bool ignore)
 {
-  tree dest, c, len, var, ret;
+  tree dest, c, len, var, ret, inner;
   unsigned HOST_WIDE_INT length, cval;
 
   if (!validate_arglist (arglist,
@@ -7939,6 +7939,15 @@ fold_builtin_memset (tree arglist, tree 
       && !POINTER_TYPE_P (TREE_TYPE (var)))
     return 0;
 
+  /* If var is a VAR_DECL or a component thereof,
+     we can use its alias set, otherwise we'd need to make
+     sure we go through alias set 0.  */
+  inner = var;
+  while (handled_component_p (inner))
+    inner = TREE_OPERAND (inner, 0);
+  if (! SSA_VAR_P (inner))
+    return 0;
+
   length = tree_low_cst (len, 1);
   if (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (var))) != length
       || get_pointer_alignment (dest, BIGGEST_ALIGNMENT) / BITS_PER_UNIT
@@ -8009,7 +8018,7 @@ fold_builtin_bzero (tree arglist, bool i
 static tree
 fold_builtin_memory_op (tree arglist, tree type, bool ignore, int endp)
 {
-  tree dest, src, len, destvar, srcvar, expr;
+  tree dest, src, len, destvar, srcvar, expr, inner;
   unsigned HOST_WIDE_INT length;
 
   if (! validate_arglist (arglist,
@@ -8050,6 +8059,15 @@ fold_builtin_memory_op (tree arglist, tr
 	  && !SCALAR_FLOAT_TYPE_P (TREE_TYPE (destvar)))
 	return 0;
 
+      /* If destvar is a VAR_DECL or a component thereof,
+	 we can use its alias set, otherwise we'd need to make
+	 sure we go through alias set 0.  */
+      inner = destvar;
+      while (handled_component_p (inner))
+	inner = TREE_OPERAND (inner, 0);
+      if (! SSA_VAR_P (inner))
+	return 0;
+
       srcvar = src;
       STRIP_NOPS (srcvar);
       if (TREE_CODE (srcvar) != ADDR_EXPR)
@@ -8064,6 +8082,15 @@ fold_builtin_memory_op (tree arglist, tr
 	  && !SCALAR_FLOAT_TYPE_P (TREE_TYPE (srcvar)))
 	return 0;
 
+      /* If srcvar is a VAR_DECL or a component thereof,
+	 we can use its alias set, otherwise we'd need to make
+	 sure we go through alias set 0.  */
+      inner = srcvar;
+      while (handled_component_p (inner))
+	inner = TREE_OPERAND (inner, 0);
+      if (! SSA_VAR_P (inner))
+	return 0;
+
       length = tree_low_cst (len, 1);
       if (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (destvar))) != length
 	  || get_pointer_alignment (dest, BIGGEST_ALIGNMENT) / BITS_PER_UNIT
--- gcc/testsuite/gcc.c-torture/execute/20060930-1.c.jj	2006-09-30 21:10:17.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/20060930-1.c	2006-09-30 21:09:33.000000000 +0200
@@ -0,0 +1,31 @@
+/* PR middle-end/29272 */
+
+extern void abort (void);
+
+struct S { struct S *s; } s;
+struct T { struct T *t; } t;
+
+static inline void
+foo (void *s)
+{
+  struct T *p = s;
+  __builtin_memcpy (&p->t, &t.t, sizeof (t.t));
+}
+
+void *
+__attribute__((noinline))
+bar (void *p, struct S *q)
+{
+  q->s = &s;
+  foo (p);
+  return q->s;
+}
+
+int
+main (void)
+{
+  t.t = &t;
+  if (bar (&s, &s) != (void *) &t)
+    abort ();
+  return 0;
+}

	Jakub


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