From: Janne Blomqvist Date: Wed, 31 Jan 2018 13:23:20 +0000 (+0200) Subject: PR 78534 Reinstate better string copy algorithm X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=9f3dcd14146c29ca01507051ae9729a0f5569173;p=gcc.git 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 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 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 3613d855a38..8f9cafc96bb 100644 --- a/gcc/fortran/ChangeLog +++ b/gcc/fortran/ChangeLog @@ -1,3 +1,11 @@ +2018-01-31 Janne Blomqvist + + 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 PR debug/84131 diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index f03aa18274d..eb359768c69 100644 --- 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 e3f0c5aafcc..a2ca981d3f5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2018-01-31 Janne Blomqvist + + 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 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 b9b70401412..d5ca573cd9f 100644 --- 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 70963bbf0e6..02e695d2d7b 100644 --- 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 73a7e7724f5..5f46cd0bccf 100644 --- 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" } }