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 10/4/06, Jakub Jelinek <jakub@redhat.com> wrote:
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?

Ping!


I believe this patch is the correct approach to fix PR29272.  At least it would
get the tramp3d tester to work again and maybe even the miscompiles of
DLV will go away (hopefully...)

Thanks,
Richard.

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]