Re: [PATCH][RFC] Fix PR61473, inline small memcpy/memmove during tree opts

[Richard Biener <rguenther@suse.de>]

On Fri, 11 Jul 2014, Richard Biener wrote:

> On Thu, 10 Jul 2014, Jakub Jelinek wrote:
> 
> > On Thu, Jul 10, 2014 at 04:30:13PM +0200, Richard Biener wrote:
> > > Compromise "hack" below.  It simply avoids the transform for
> > > sources that c_strlen can compute a length of.  That "fixes" all
> > > strlenopt testcase apart from strlenopt-8.c which does
> > > memcpy (, flag ? "a" : "b"); which then still folds
> > > during gimplification.  I have XFAILed that.
> > > 
> > > I've tried to comb my way through strlenopt but failed to quickly
> > > add support for generic stores (it has very rough support for
> > > char stores, see also PR61773).
> > > 
> > > Does the below look ok?
> > 
> > I can look at the tree-ssa-strlen.c stuff and removing the !c_strlen
> > hack incrementally.
> 
> Ok, I'll bootstrap/test the patch then and will commit if there are
> no issues left.

This is what I have applied.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2014-07-11  Richard Biener  <rguenther@suse.de>

	PR middle-end/61473
	* builtins.c (fold_builtin_memory_op): Inline memory moves
	that can be implemented with a single load followed by a
	single store.
	(c_strlen): Only warn when only_value is not 2.

	* gcc.dg/memmove-4.c: New testcase.
	* gcc.dg/strlenopt-8.c: XFAIL.
	* gfortran.dg/coarray_lib_realloc_1.f90: Adjust.

Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c.orig	2014-07-11 09:54:49.844932232 +0200
--- gcc/builtins.c	2014-07-11 13:15:35.297102917 +0200
*************** get_pointer_alignment (tree exp)
*** 535,540 ****
--- 535,544 ----
     len = c_strlen (src, 1); if (len) expand_expr (len, ...); would not
     evaluate the side-effects.
  
+    If ONLY_VALUE is two then we do not emit warnings about out-of-bound
+    accesses.  Note that this implies the result is not going to be emitted
+    into the instruction stream.
+ 
     The value returned is of type `ssizetype'.
  
     Unfortunately, string_constant can't access the values of const char
*************** c_strlen (tree src, int only_value)
*** 606,612 ****
  
    /* If the offset is known to be out of bounds, warn, and call strlen at
       runtime.  */
!   if (offset < 0 || offset > max)
      {
       /* Suppress multiple warnings for propagated constant strings.  */
        if (! TREE_NO_WARNING (src))
--- 610,617 ----
  
    /* If the offset is known to be out of bounds, warn, and call strlen at
       runtime.  */
!   if (only_value != 2
!       && (offset < 0 || offset > max))
      {
       /* Suppress multiple warnings for propagated constant strings.  */
        if (! TREE_NO_WARNING (src))
*************** fold_builtin_memory_op (location_t loc,
*** 8637,8647 ****
        unsigned int src_align, dest_align;
        tree off0;
  
!       if (endp == 3)
  	{
! 	  src_align = get_pointer_alignment (src);
! 	  dest_align = get_pointer_alignment (dest);
  
  	  /* Both DEST and SRC must be pointer types.
  	     ??? This is what old code did.  Is the testing for pointer types
  	     really mandatory?
--- 8642,8698 ----
        unsigned int src_align, dest_align;
        tree off0;
  
!       /* Build accesses at offset zero with a ref-all character type.  */
!       off0 = build_int_cst (build_pointer_type_for_mode (char_type_node,
! 							 ptr_mode, true), 0);
! 
!       /* If we can perform the copy efficiently with first doing all loads
!          and then all stores inline it that way.  Currently efficiently
! 	 means that we can load all the memory into a single integer
! 	 register which is what MOVE_MAX gives us.  */
!       src_align = get_pointer_alignment (src);
!       dest_align = get_pointer_alignment (dest);
!       if (tree_fits_uhwi_p (len)
! 	  && compare_tree_int (len, MOVE_MAX) <= 0
! 	  /* ???  Don't transform copies from strings with known length this
! 	     confuses the tree-ssa-strlen.c.  This doesn't handle
! 	     the case in gcc.dg/strlenopt-8.c which is XFAILed for that
! 	     reason.  */
! 	  && !c_strlen (src, 2))
  	{
! 	  unsigned ilen = tree_to_uhwi (len);
! 	  if (exact_log2 (ilen) != -1)
! 	    {
! 	      tree type = lang_hooks.types.type_for_size (ilen * 8, 1);
! 	      if (type
! 		  && TYPE_MODE (type) != BLKmode
! 		  && (GET_MODE_SIZE (TYPE_MODE (type)) * BITS_PER_UNIT
! 		      == ilen * 8)
! 		  /* If the pointers are not aligned we must be able to
! 		     emit an unaligned load.  */
! 		  && ((src_align >= GET_MODE_ALIGNMENT (TYPE_MODE (type))
! 		       && dest_align >= GET_MODE_ALIGNMENT (TYPE_MODE (type)))
! 		      || !SLOW_UNALIGNED_ACCESS (TYPE_MODE (type),
! 						 MIN (src_align, dest_align))))
! 		{
! 		  tree srctype = type;
! 		  tree desttype = type;
! 		  if (src_align < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
! 		    srctype = build_aligned_type (type, src_align);
! 		  if (dest_align < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
! 		    desttype = build_aligned_type (type, dest_align);
! 		  if (!ignore)
! 		    dest = builtin_save_expr (dest);
! 		  expr = build2 (MODIFY_EXPR, type,
! 				 fold_build2 (MEM_REF, desttype, dest, off0),
! 				 fold_build2 (MEM_REF, srctype, src, off0));
! 		  goto done;
! 		}
! 	    }
! 	}
  
+       if (endp == 3)
+ 	{
  	  /* Both DEST and SRC must be pointer types.
  	     ??? This is what old code did.  Is the testing for pointer types
  	     really mandatory?
*************** fold_builtin_memory_op (location_t loc,
*** 8818,8827 ****
        if (!ignore)
          dest = builtin_save_expr (dest);
  
-       /* Build accesses at offset zero with a ref-all character type.  */
-       off0 = build_int_cst (build_pointer_type_for_mode (char_type_node,
- 							 ptr_mode, true), 0);
- 
        destvar = dest;
        STRIP_NOPS (destvar);
        if (TREE_CODE (destvar) == ADDR_EXPR
--- 8869,8874 ----
*************** fold_builtin_memory_op (location_t loc,
*** 8888,8893 ****
--- 8935,8941 ----
        expr = build2 (MODIFY_EXPR, TREE_TYPE (destvar), destvar, srcvar);
      }
  
+ done:
    if (ignore)
      return expr;
  
Index: gcc/testsuite/gcc.dg/memmove-4.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/memmove-4.c	2014-07-11 12:34:10.168274016 +0200
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fdump-tree-optimized" } */
+ 
+ typedef int w __attribute__((mode(word)));
+ 
+ void b(char *a, char *b, int i)
+ {
+   __builtin_memmove (&a[i], &b[i], sizeof(w));
+ }
+ 
+ /* { dg-final { scan-tree-dump-not "memmove" "optimized" { xfail { ! non_strict_align } } } } */
+ /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: gcc/testsuite/gcc.dg/strlenopt-8.c
===================================================================
*** gcc/testsuite/gcc.dg/strlenopt-8.c.orig	2014-07-11 09:54:49.844932232 +0200
--- gcc/testsuite/gcc.dg/strlenopt-8.c	2014-07-11 12:34:10.168274016 +0200
*************** main ()
*** 43,50 ****
    return 0;
  }
  
! /* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 4 "strlen" } } */
  /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
  /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
  /* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
--- 43,50 ----
    return 0;
  }
  
! /* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" { xfail *-*-* } } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 4 "strlen" { xfail *-*-* } } } */
  /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
  /* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
  /* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
Index: gcc/testsuite/gfortran.dg/coarray_lib_realloc_1.f90
===================================================================
*** gcc/testsuite/gfortran.dg/coarray_lib_realloc_1.f90.orig	2014-07-11 09:54:49.844932232 +0200
--- gcc/testsuite/gfortran.dg/coarray_lib_realloc_1.f90	2014-07-11 12:34:10.178274015 +0200
*************** end
*** 30,35 ****
  ! { dg-final { scan-tree-dump-times "__builtin_malloc" 1 "original" } }
  
  ! But copy "ii" and "CAF":
! ! { dg-final { scan-tree-dump-times "__builtin_memcpy" 2 "original" } }
  
  ! { dg-final { cleanup-tree-dump "original" } }
--- 30,35 ----
  ! { dg-final { scan-tree-dump-times "__builtin_malloc" 1 "original" } }
  
  ! But copy "ii" and "CAF":
! ! { dg-final { scan-tree-dump-times "__builtin_memcpy|= MEM" 2 "original" } }
  
  ! { dg-final { cleanup-tree-dump "original" } }