PR 78534 Reinstate better string copy algorithm

As part of the change to larger character lengths, the string copy
algorithm was temporarily pessimized to get around some spurious
-Wstringop-overflow warnings.  Having tried a number of variations of
this algorithm I have managed to get it down to one spurious warning,
only with -O1 optimization, in the testsuite.  This patch reinstates
the optimized variant and modifies this one testcase to ignore the
warning.

Regtested on x86_64-pc-linux-gnu.

gcc/fortran/ChangeLog:

2018-01-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR fortran/78534
	* trans-expr.c (fill_with_spaces): Use memset instead of
	generating loop.
	(gfc_trans_string_copy): Improve opportunity to use builtins with
	constant lengths.

gcc/testsuite/ChangeLog:

2018-01-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR fortran/78534
	* gfortran.dg/allocate_deferred_char_scalar_1.f03: Prune
	-Wstringop-overflow warnings due to spurious warning with -O1.
	* gfortran.dg/char_cast_1.f90: Update dump scan pattern.
	* gfortran.dg/transfer_intrinsic_1.f90: Likewise.

From-SVN: r257233

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 3613d855a388110c9040bb8ff9fb9c6d4b3614c4..8f9cafc96bb56ef5faaf3186a41f95ad7e3be5e1 100644 (file)
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,11 @@
+2018-01-31  Janne Blomqvist  <jb@gcc.gnu.org>
+
+       PR fortran/78534
+       * trans-expr.c (fill_with_spaces): Use memset instead of
+       generating loop.
+       (gfc_trans_string_copy): Improve opportunity to use builtins with
+       constant lengths.
+
 2018-01-30  Jakub Jelinek  <jakub@redhat.com>
 
        PR debug/84131

diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index f03aa18274d51d91d37476d2c1cafcdb56152ec5..eb359768c69faeb2da83a96a27b8169dccb2a000 100644 (file)
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -6411,8 +6411,6 @@ fill_with_spaces (tree start, tree type, tree size)
   tree i, el, exit_label, cond, tmp;
 
   /* For a simple char type, we can call memset().  */
-  /* TODO: This code does work and is potentially more efficient, but
-     causes spurious -Wstringop-overflow warnings.
   if (compare_tree_int (TYPE_SIZE_UNIT (type), 1) == 0)
     return build_call_expr_loc (input_location,
                            builtin_decl_explicit (BUILT_IN_MEMSET),
@@ -6420,7 +6418,6 @@ fill_with_spaces (tree start, tree type, tree size)
                            build_int_cst (gfc_get_int_type (gfc_c_int_kind),
                                           lang_hooks.to_target_charset (' ')),
                                fold_convert (size_type_node, size));
-  */
 
   /* Otherwise, we use a loop:
        for (el = start, i = size; i > 0; el--, i+= TYPE_SIZE_UNIT (type))
@@ -6526,11 +6523,20 @@ gfc_trans_string_copy (stmtblock_t * block, tree dlength, tree dest,
 
   /* The string copy algorithm below generates code like
 
-     if (dlen > 0) {
-         memmove (dest, src, min(dlen, slen));
-         if (slen < dlen)
-             memset(&dest[slen], ' ', dlen - slen);
-     }
+     if (destlen > 0)
+       {
+         if (srclen < destlen)
+           {
+             memmove (dest, src, srclen);
+             // Pad with spaces.
+             memset (&dest[srclen], ' ', destlen - srclen);
+           }
+         else
+           {
+             // Truncate if too long.
+             memmove (dest, src, destlen);
+           }
+       }
   */
 
   /* Do nothing if the destination length is zero.  */
@@ -6559,21 +6565,16 @@ gfc_trans_string_copy (stmtblock_t * block, tree dlength, tree dest,
   else
     src = gfc_build_addr_expr (pvoid_type_node, src);
 
-  /* First do the memmove. */
-  tmp2 = fold_build2_loc (input_location, MIN_EXPR, TREE_TYPE (dlen), dlen,
-                         slen);
-  tmp2 = build_call_expr_loc (input_location,
-                             builtin_decl_explicit (BUILT_IN_MEMMOVE),
-                             3, dest, src,
-                             fold_convert (size_type_node, tmp2));
-  stmtblock_t tmpblock2;
-  gfc_init_block (&tmpblock2);
-  gfc_add_expr_to_block (&tmpblock2, tmp2);
-
-  /* If the destination is longer, fill the end with spaces.  */
+  /* Truncate string if source is too long.  */
   cond2 = fold_build2_loc (input_location, LT_EXPR, logical_type_node, slen,
                           dlen);
 
+  /* Copy and pad with spaces.  */
+  tmp3 = build_call_expr_loc (input_location,
+                             builtin_decl_explicit (BUILT_IN_MEMMOVE),
+                             3, dest, src,
+                             fold_convert (size_type_node, slen));
+
   /* Wstringop-overflow appears at -O3 even though this warning is not
      explicitly available in fortran nor can it be switched off. If the
      source length is a constant, its negative appears as a very large
@@ -6588,14 +6589,19 @@ gfc_trans_string_copy (stmtblock_t * block, tree dlength, tree dest,
   tmp4 = fill_with_spaces (tmp4, chartype, tmp);
 
   gfc_init_block (&tempblock);
+  gfc_add_expr_to_block (&tempblock, tmp3);
   gfc_add_expr_to_block (&tempblock, tmp4);
   tmp3 = gfc_finish_block (&tempblock);
 
+  /* The truncated memmove if the slen >= dlen.  */
+  tmp2 = build_call_expr_loc (input_location,
+                             builtin_decl_explicit (BUILT_IN_MEMMOVE),
+                             3, dest, src,
+                             fold_convert (size_type_node, dlen));
+
   /* The whole copy_string function is there.  */
   tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond2,
-                        tmp3, build_empty_stmt (input_location));
-  gfc_add_expr_to_block (&tmpblock2, tmp);
-  tmp = gfc_finish_block (&tmpblock2);
+                        tmp3, tmp2);
   tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond, tmp,
                         build_empty_stmt (input_location));
   gfc_add_expr_to_block (block, tmp);

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index e3f0c5aafccfa52265c9d5d77f9903aa3f1af627..a2ca981d3f537d849cac9b7102bd54c0f4b5068b 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2018-01-31  Janne Blomqvist  <jb@gcc.gnu.org>
+
+       PR fortran/78534
+       * gfortran.dg/allocate_deferred_char_scalar_1.f03: Prune
+       -Wstringop-overflow warnings due to spurious warning with -O1.
+       * gfortran.dg/char_cast_1.f90: Update dump scan pattern.
+       * gfortran.dg/transfer_intrinsic_1.f90: Likewise.
+
 2018-01-31  Richard Biener  <rguenther@suse.de>
 
        PR tree-optimization/84132

diff --git a/gcc/testsuite/gfortran.dg/allocate_deferred_char_scalar_1.f03 b/gcc/testsuite/gfortran.dg/allocate_deferred_char_scalar_1.f03
index b9b70401412c2e203083e98d8e642289410ada94..d5ca573cd9fa1a35a02fa468c1e972e12d4b901e 100644 (file)
--- a/gcc/testsuite/gfortran.dg/allocate_deferred_char_scalar_1.f03
+++ b/gcc/testsuite/gfortran.dg/allocate_deferred_char_scalar_1.f03
@@ -265,3 +265,5 @@ contains
      if(len(p4) /= 3) call abort()
   end subroutine source3
 end program test
+! Spurious -Wstringop-overflow warning with -O1
+! { dg-prune-output "\\\[-Wstringop-overflow=]" } 

diff --git a/gcc/testsuite/gfortran.dg/char_cast_1.f90 b/gcc/testsuite/gfortran.dg/char_cast_1.f90
index 70963bbf0e6cb70f4e0ad188269a9cb47a08145f..02e695d2d7b738996db6c756d90fc79ee64b9daf 100644 (file)
--- a/gcc/testsuite/gfortran.dg/char_cast_1.f90
+++ b/gcc/testsuite/gfortran.dg/char_cast_1.f90
@@ -25,6 +25,6 @@
     return
   end function Upper
 end
-! The sign that all is well is that [S.10][1] appears twice.
-! Platform dependent variations are [S$10][1], [__S_10][1], [S___10][1]
-! { dg-final { scan-tree-dump-times "10\\\]\\\[1\\\]" 2 "original" } }
+! The sign that all is well is that [S.6][1] appears twice.
+! Platform dependent variations are [S$6][1], [__S_6][1], [S___6][1]
+! { dg-final { scan-tree-dump-times "6\\\]\\\[1\\\]" 2 "original" } }

diff --git a/gcc/testsuite/gfortran.dg/transfer_intrinsic_1.f90 b/gcc/testsuite/gfortran.dg/transfer_intrinsic_1.f90
index 73a7e7724f5b08c790a53625e994afd57f2aeb2d..5f46cd0bccf3a4bf9a53b8c2908196d26dfe51d1 100644 (file)
--- a/gcc/testsuite/gfortran.dg/transfer_intrinsic_1.f90
+++ b/gcc/testsuite/gfortran.dg/transfer_intrinsic_1.f90
@@ -14,4 +14,4 @@ subroutine BytesToString(bytes, string)
     character(len=*) :: string
     string = transfer(bytes, string)
   end subroutine
-! { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "original" } }
+! { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "original" } }