On 27 February 2017 at 00:10, Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> wrote: > On 26 February 2017 at 05:16, Martin Sebor <mse...@gmail.com> wrote: >> On 02/25/2017 11:54 AM, Prathamesh Kulkarni wrote: >>> >>> On 25 February 2017 at 14:43, Marc Glisse <marc.gli...@inria.fr> wrote: >>>> >>>> On Sat, 25 Feb 2017, Prathamesh Kulkarni wrote: >>>> >>>>> Hi, >>>>> The attached patch deletes calls to strdup, strndup if it's >>>>> return-value is unused, >>>>> and same for realloc if the first arg is NULL. >>>>> Bootstrap+tested on x86_64-unknown-linux-gnu. >>>>> OK for GCC 8 ? >>>> >>>> >>>> >>>> Instead of specializing realloc(0,*) wherever we can perform the same >>>> optimization as with malloc, wouldn't it be better to optimize: >>>> realloc(0,n) -> malloc(n) >>>> and let the malloc optimizations happen? >>> >>> Thanks for the suggestions. In the attached patch, realloc (0, n) is >>> folded to malloc (n). >>> Bootstrap+test in progress on x86_64-unknown-linux-gnu. >>> Does the patch look OK ? >> >> >> Although it's not part of the bug report, I wonder if a complete >> patch should also extend the malloc/free DCE to str{,n}dup/free >> calls and eliminate pairs like these: >> >> void f (const char *s) >> { >> char *p = strdup (src); >> free (p); >> } >> >> (That seems to be just a matter of adding a couple of conditionals >> for BUILT_IN_STR{,N}DUP in propagate_necessity). > Hi Martin, > Thanks for the suggestions, I have updated the patch accordingly. > Does it look OK ? Hi, The attached patch for PR79697 passes bootstrap+test on x86_64 and cross-tested on arm*-*-* and aarch64*-*-*. As per the previous suggestions, the patch folds realloc (0, n) -> malloc (n). Is it OK for trunk ?
Thanks, Prathamesh >> >> Martin >> >> PS Another optimization, though one that's most likely outside >> the scope of this patch, is to eliminate all of the following: >> >> void f (const char *s, char *s2) >> { >> char *p = strdup (s); >> strcpy (p, s2); >> free (p); >> } >> >> as is done in: >> >> void f (unsigned n, const char *s) >> { >> char *p = malloc (n); >> memcpy (p, s, n); >> free (p); >> } >> > Hmm, dse1 detects that the store to p in memcpy() is dead and deletes the > call. > cddce1 then removes calls to malloc and free thus making the function empty. > It doesn't work if memcpy is replaced by strcpy: > > void f (unsigned n, char *s2) > { > char *p = __builtin_malloc (n); > __builtin_strcpy (p, s2); > __builtin_free (p); > } > > I suppose strcpy should be special-cased too in dse similar to memcpy ? > > Thanks, > Prathamesh
2017-04-24 Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> PR tree-optimization/79697 * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Check if callee is BUILT_IN_STRDUP, BUILT_IN_STRNDUP, BUILT_IN_REALLOC. (propagate_necessity): Check if def_callee is BUILT_IN_STRDUP or BUILT_IN_STRNDUP. * gimple-fold.c (gimple_fold_builtin_realloc): New function. (gimple_fold_builtin): Call gimple_fold_builtin_realloc. testsuite/ * gcc.dg/tree-ssa/pr79697.c: New test. diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index a75dd91..e6eceea 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -3251,6 +3251,28 @@ gimple_fold_builtin_acc_on_device (gimple_stmt_iterator *gsi, tree arg0) return true; } +/* Fold realloc (0, n) -> malloc (n). */ + +static bool +gimple_fold_builtin_realloc (gimple_stmt_iterator *gsi) +{ + gimple *stmt = gsi_stmt (*gsi); + tree arg = gimple_call_arg (stmt, 0); + tree size = gimple_call_arg (stmt, 1); + + if (operand_equal_p (arg, null_pointer_node, 0)) + { + tree fn_malloc = builtin_decl_implicit (BUILT_IN_MALLOC); + if (fn_malloc) + { + gcall *repl = gimple_build_call (fn_malloc, 1, size); + replace_call_with_call_and_fold (gsi, repl); + return true; + } + } + return false; +} + /* Fold the non-target builtin at *GSI and return whether any simplification was made. */ @@ -3409,6 +3431,9 @@ gimple_fold_builtin (gimple_stmt_iterator *gsi) case BUILT_IN_ACC_ON_DEVICE: return gimple_fold_builtin_acc_on_device (gsi, gimple_call_arg (stmt, 0)); + case BUILT_IN_REALLOC: + return gimple_fold_builtin_realloc (gsi); + default:; } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c b/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c new file mode 100644 index 0000000..d4f6473 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-gimple -fdump-tree-cddce-details -fdump-tree-optimized" } */ + +void f(void) +{ + __builtin_strdup ("abc"); +} + +void g(void) +{ + __builtin_strndup ("abc", 3); +} + +void h(void) +{ + __builtin_realloc (0, 10); +} + +void k(void) +{ + char *p = __builtin_strdup ("abc"); + __builtin_free (p); + + char *q = __builtin_strndup ("abc", 3); + __builtin_free (q); +} + +/* { dg-final { scan-tree-dump "Deleting : __builtin_strdup" "cddce1" } } */ +/* { dg-final { scan-tree-dump "Deleting : __builtin_strndup" "cddce1" } } */ +/* { dg-final { scan-tree-dump "__builtin_malloc" "gimple" } } */ +/* { dg-final { scan-tree-dump-not "__builtin_strdup" "optimized" } } */ +/* { dg-final { scan-tree-dump-not "__builtin_strndup" "optimized" } } */ +/* { dg-final { scan-tree-dump-not "__builtin_free" "optimized" } } */ diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index 5ebe57b..e17659d 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -233,6 +233,8 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) case BUILT_IN_CALLOC: case BUILT_IN_ALLOCA: case BUILT_IN_ALLOCA_WITH_ALIGN: + case BUILT_IN_STRDUP: + case BUILT_IN_STRNDUP: return; default:; @@ -780,7 +782,9 @@ propagate_necessity (bool aggressive) && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_ALIGNED_ALLOC || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC - || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC)) + || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC + || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_STRDUP + || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_STRNDUP)) { gimple *bounds_def_stmt; tree bounds;