Re: [PATCH] Fix memcpy folding (PR c++/38705)

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Sat, Jan 3, 2009 at 12:52 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> If one of the vars whose address is passed to memcpy is e.g. const
> qualified, we shouldn't attempt to write through say *(const int *)ptr.
> Similarly for restrict, for volatile we IMHO just don't want to optimize.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux.  Ok for trunk?

While I don't see a reason for restricting this for volatile I don't feel strong
about this either (people cannot expect volatile semantics from calling
memcpy with volatile pointers anyway).

So, this is ok with or without the volatile restriction.

Thanks,
Richard.

> 2009-01-03  Jakub Jelinek  <jakub@redhat.com>
>
>        PR c++/38705
>        * builtins.c (fold_builtin_memory_op): Give up if either operand
>        is volatile.  Set srctype or desttype to non-qualified version
>        of the other type.
>
>        * g++.dg/torture/pr38705.C: New test.
>
> --- gcc/builtins.c.jj   2009-01-02 16:06:03.000000000 +0100
> +++ gcc/builtins.c      2009-01-02 20:59:18.000000000 +0100
> @@ -8907,7 +8907,9 @@ fold_builtin_memory_op (tree dest, tree
>          || !TYPE_SIZE_UNIT (srctype)
>          || !TYPE_SIZE_UNIT (desttype)
>          || TREE_CODE (TYPE_SIZE_UNIT (srctype)) != INTEGER_CST
> -         || TREE_CODE (TYPE_SIZE_UNIT (desttype)) != INTEGER_CST)
> +         || TREE_CODE (TYPE_SIZE_UNIT (desttype)) != INTEGER_CST
> +         || TYPE_VOLATILE (srctype)
> +         || TYPE_VOLATILE (desttype))
>        return NULL_TREE;
>
>       src_align = get_pointer_alignment (src, BIGGEST_ALIGNMENT);
> @@ -8924,7 +8926,7 @@ fold_builtin_memory_op (tree dest, tree
>        {
>          srcvar = build_fold_indirect_ref (src);
>          if (TREE_THIS_VOLATILE (srcvar))
> -           srcvar = NULL_TREE;
> +           return NULL_TREE;
>          else if (!tree_int_cst_equal (lang_hooks.expr_size (srcvar), len))
>            srcvar = NULL_TREE;
>          /* With memcpy, it is possible to bypass aliasing rules, so without
> @@ -8942,7 +8944,7 @@ fold_builtin_memory_op (tree dest, tree
>        {
>          destvar = build_fold_indirect_ref (dest);
>          if (TREE_THIS_VOLATILE (destvar))
> -           destvar = NULL_TREE;
> +           return NULL_TREE;
>          else if (!tree_int_cst_equal (lang_hooks.expr_size (destvar), len))
>            destvar = NULL_TREE;
>          else if (!var_decl_component_p (destvar))
> @@ -8958,7 +8960,7 @@ fold_builtin_memory_op (tree dest, tree
>          if (TREE_ADDRESSABLE (TREE_TYPE (destvar)))
>            return NULL_TREE;
>
> -         srctype = desttype;
> +         srctype = build_qualified_type (desttype, 0);
>          if (src_align < (int) TYPE_ALIGN (srctype))
>            {
>              if (AGGREGATE_TYPE_P (srctype)
> @@ -8980,7 +8982,7 @@ fold_builtin_memory_op (tree dest, tree
>          if (TREE_ADDRESSABLE (TREE_TYPE (srcvar)))
>            return NULL_TREE;
>
> -         desttype = srctype;
> +         desttype = build_qualified_type (srctype, 0);
>          if (dest_align < (int) TYPE_ALIGN (desttype))
>            {
>              if (AGGREGATE_TYPE_P (desttype)
> --- gcc/testsuite/g++.dg/torture/pr38705.C.jj   2009-01-02 21:04:57.000000000 +0100
> +++ gcc/testsuite/g++.dg/torture/pr38705.C      2009-01-02 21:04:28.000000000 +0100
> @@ -0,0 +1,27 @@
> +// PR c++/38705
> +// { dg-do compile }
> +
> +typedef int T;
> +typedef __SIZE_TYPE__ size_t;
> +extern "C" void *memcpy (void *, const void *, size_t);
> +
> +void
> +foo (char *p, const int q)
> +{
> +  memcpy (p, &q, sizeof (int));
> +}
> +
> +struct S
> +{
> +  T t;
> +  int u;
> +  int bar () const;
> +  template <class T> void foo (const T &x) const {}
> +};
> +
> +int
> +S::bar () const
> +{
> +  foo (u);
> +  foo (t);
> +}
>
>        Jakub
>