Re: [PATCH] PR35258: Prevent CSE from generating overlapping memmoves

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

On Mon, Feb 25, 2008 at 2:24 PM, Andreas Krebbel
<Andreas.Krebbel@de.ibm.com> wrote:
> Hi,
>
>
>  > It looks like you can use nonoverlapping_memrefs_p from alias.c
>  > instead of adding cse_mem_cannot_overlap_p?
>
>  I thought that one is only supposed to be used from within alias.c but
>  actually I don't see a reason for that.
>
>  So here is the patch.
>
>  Bootstrapped and regtested on x86_64, s390 and s390x.
>
>  OK for mainline and 4.3?

Ok for mainline and 4.3.1 (please wait until after 4.3.0 is out).

Thanks,
Richard.

>  Bye,
>
>  -Andreas-
>
>  2008-02-25  Andreas Krebbel  <krebbel1@de.ibm.com>
>
>         * cse.c (cse_insn): Avoid creation of overlapping MEMs.
>         * alias.c (nonoverlapping_memrefs_p): Export for use in other modules.
>         * alias.h (nonoverlapping_memrefs_p): Likewise.
>
>  2008-02-25  Andreas Krebbel  <krebbel1@de.ibm.com>
>
>
>         * gcc.dg/pr35258.c: New testcase.
>
>  Index: gcc/cse.c
>  ===================================================================
>  *** gcc/cse.c.orig      2008-02-22 10:46:18.000000000 +0100
>  --- gcc/cse.c   2008-02-25 09:15:34.000000000 +0100
>
> *************** cse_insn (rtx insn, rtx libcall_insn)
>  *** 4753,4758 ****
>  --- 4753,4775 ----
>
>               src_elt_cost = MAX_COST;
>             }
>
>  +         /* Avoid creation of overlapping memory moves.  */
>  +         if (MEM_P (trial) && MEM_P (SET_DEST (sets[i].rtl)))
>  +           {
>  +             rtx src, dest;
>  +
>  +             /* BLKmode moves are not handled by cse anyway.  */
>  +             if (GET_MODE (trial) == BLKmode)
>  +               break;
>  +
>  +             src = canon_rtx (trial);
>  +             dest = canon_rtx (SET_DEST (sets[i].rtl));
>  +
>  +             if (!MEM_P (src) || !MEM_P (dest)
>  +                 || !nonoverlapping_memrefs_p (src, dest))
>
>
> +               break;
>  +           }
>  +
>           /* We don't normally have an insn matching (set (pc) (pc)), so
>              check for this separately here.  We will delete such an
>              insn below.
>  Index: gcc/testsuite/gcc.dg/pr35258.c
>  ===================================================================
>  *** /dev/null   1970-01-01 00:00:00.000000000 +0000
>  --- gcc/testsuite/gcc.dg/pr35258.c      2008-02-22 10:47:55.000000000 +0100
>  ***************
>  *** 0 ****
>  --- 1,28 ----
>  + /* { dg-do run } */
>  + /* { dg-options "-O1" } */
>  +
>  + extern void *memcpy (void *, const void *, __SIZE_TYPE__);
>  + extern int memcmp (const void *, const void *, __SIZE_TYPE__);
>  + extern void abort(void);
>  +
>  + char str[9] = "1234";
>  +
>  + void
>  + bar (void)
>  + {
>  +   unsigned int temp;
>  +   char *p = &str[2];
>  +
>  +   memcpy (&temp, &str[1], 4);
>  +   memcpy (p, &temp, 4);
>  +   str[1] = '.';
>  + }
>  +
>  + int main()
>  + {
>  +   bar();
>  +   if (memcmp (str, "1.234", 5) != 0)
>  +     abort ();
>  +
>  +   return 0;
>  + }
>  Index: gcc/alias.c
>  ===================================================================
>  *** gcc/alias.c.orig    2007-10-26 10:35:40.000000000 +0200
>  --- gcc/alias.c 2008-02-25 09:11:51.000000000 +0100
>  *************** static int aliases_everything_p (const_r
>  *** 164,170 ****
>   static bool nonoverlapping_component_refs_p (const_tree, const_tree);
>   static tree decl_for_component_ref (tree);
>   static rtx adjust_offset_for_component_ref (tree, rtx);
>  - static int nonoverlapping_memrefs_p (const_rtx, const_rtx);
>   static int write_dependence_p (const_rtx, const_rtx, int);
>
>   static void memory_modified_1 (rtx, const_rtx, void *);
>  --- 164,169 ----
>  *************** adjust_offset_for_component_ref (tree x,
>  *** 1976,1982 ****
>   /* Return nonzero if we can determine the exprs corresponding to memrefs
>      X and Y and they do not overlap.  */
>
>  ! static int
>   nonoverlapping_memrefs_p (const_rtx x, const_rtx y)
>   {
>     tree exprx = MEM_EXPR (x), expry = MEM_EXPR (y);
>  --- 1975,1981 ----
>   /* Return nonzero if we can determine the exprs corresponding to memrefs
>      X and Y and they do not overlap.  */
>
>  ! int
>   nonoverlapping_memrefs_p (const_rtx x, const_rtx y)
>   {
>     tree exprx = MEM_EXPR (x), expry = MEM_EXPR (y);
>  Index: gcc/alias.h
>  ===================================================================
>  *** gcc/alias.h.orig    2007-08-28 08:51:49.000000000 +0200
>  --- gcc/alias.h 2008-02-25 09:10:48.000000000 +0100
>  *************** extern alias_set_type get_varargs_alias_
>  *** 28,33 ****
>  --- 28,34 ----
>   extern alias_set_type get_frame_alias_set (void);
>   extern bool component_uses_parent_alias_set (const_tree);
>   extern bool alias_set_subset_of (alias_set_type, alias_set_type);
>  + extern int nonoverlapping_memrefs_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
>