On Thu, Oct 31, 2024 at 9:08 AM Jan Hubicka <hubi...@ucw.cz> wrote: > > Hi, > this patch lets us to remove > void *mem = __builtin_malloc (s); > if (!mem) > __builtin_abort (); > __builtin_free (mem); > > Where we previously stopped on "if (!mem)" marking __builtin_malloc necessary. > This is done by matching such conditional in tree-ssa-dce.cc and playing same > game > as we do for free functions. We consider if itself necessary (since it does > control flow) but we do not propagate necessity over the SSA name. > > In removal stage we check whether SSA name is needed and if not, we replace > it by constant 1, which makes the conditonal to be folded as constant in > cfgccleanup. > > While working on this, I refactored the code a bit to avoid duplicate matching > of allocation functions which made me notice that we handled strdup/strndup > just > half way and this prevented us from removing it, which is also fixed and > tested > in the testcase. > > Note that testcase pr68078.f90 checks that failed allocation is reported > when memory limits are set. We no longer report it since the memory is > unused and that is why I needed to disable DCE. > > I alos tested that with Jakub's patch we now eliminate new operator with > std::nothrow_t. > > Bootstrapped/regtested x86_64-linux, OK?
Note the request for this optimization is recorded in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19831#c20 so it should have `PR tree-optimization/19831` on the changelog at least. Also how does this change affect the other tests in that comment? Like the alloc_fails/alloc_success one? or the realloc one? Thanks, Andrew > > gcc/ChangeLog: > > * tree-ssa-dce.cc (is_removable_allocation_p): Break out from > ... > (mark_stmt_if_obviously_necessary): ... here; also check that > operator new satisfies gimple_call_from_new_or_delete. > (checks_return_value_of_removable_allocation_p): New Function. > (mark_all_reaching_defs_necessary_1): add missing case ofr > STRDUP and STRNDUP > (propagate_necessity): Use is_removable_allocation_p and > checks_return_value_of_removable_allocation_p. > (eliminate_unnecessary_stmts): Update conditionals that use > removed allocation; use is_removable_allocation_p. > > gcc/testsuite/ChangeLog: > > * g++.dg/cdce3.C: Disable allocation DCE. > * g++.dg/tree-ssa/pr19476-1.C: Likewise. > * g++.dg/tree-ssa/pr19476-2.C: Likewise. > * g++.dg/tree-ssa/pr19476-3.C: Likewise. > * g++.dg/tree-ssa/pr19476-4.C: Likewise. > * gcc.dg/tree-ssa/pr19831-3.c: Test is now fully optiimzed. > * gfortran.dg/pr68078.f90: Disable DCE. > * gcc.dg/tree-ssa/dce-3.c: New test. > > diff --git a/gcc/testsuite/g++.dg/cdce3.C b/gcc/testsuite/g++.dg/cdce3.C > index 2543317f6b2..9aecf58a8b8 100644 > --- a/gcc/testsuite/g++.dg/cdce3.C > +++ b/gcc/testsuite/g++.dg/cdce3.C > @@ -1,6 +1,6 @@ > /* { dg-do run } */ > /* { dg-require-effective-target c99_runtime } */ > -/* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -lm" } */ > +/* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -lm > -fno-allocation-dce" } */ > /* { dg-additional-options "-DLARGE_LONG_DOUBLE" { target large_long_double > } } */ > /* { dg-additional-options "-DGNU_EXTENSION" { target pow10 } } */ > /* { dg-add-options ieee } */ > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19476-1.C > b/gcc/testsuite/g++.dg/tree-ssa/pr19476-1.C > index 25867185648..d17be88642f 100644 > --- a/gcc/testsuite/g++.dg/tree-ssa/pr19476-1.C > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr19476-1.C > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O -fdump-tree-dom2 -fdelete-null-pointer-checks" } */ > +/* { dg-options "-O -fdump-tree-dom2 -fdelete-null-pointer-checks > -fno-allocation-dce" } */ > /* { dg-skip-if "" keeps_null_pointer_checks } */ > > // See pr19476-5.C for a version without including <new>. > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19476-2.C > b/gcc/testsuite/g++.dg/tree-ssa/pr19476-2.C > index 0f657d5bd0c..aab7d04ab92 100644 > --- a/gcc/testsuite/g++.dg/tree-ssa/pr19476-2.C > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr19476-2.C > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-optimized -fdelete-null-pointer-checks" } */ > +/* { dg-options "-O2 -fdump-tree-optimized -fdelete-null-pointer-checks > -fno-allocation-dce" } */ > /* { dg-skip-if "" keeps_null_pointer_checks } */ > > #include <new> > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19476-3.C > b/gcc/testsuite/g++.dg/tree-ssa/pr19476-3.C > index fd5117fceb8..0894178c470 100644 > --- a/gcc/testsuite/g++.dg/tree-ssa/pr19476-3.C > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr19476-3.C > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */ > +/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized -fno-allocation-dce" > } */ > > #include <new> > > diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr19476-4.C > b/gcc/testsuite/g++.dg/tree-ssa/pr19476-4.C > index 5f137391cc6..e5c216a64fa 100644 > --- a/gcc/testsuite/g++.dg/tree-ssa/pr19476-4.C > +++ b/gcc/testsuite/g++.dg/tree-ssa/pr19476-4.C > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O3 -fno-delete-null-pointer-checks -fdump-tree-optimized" > } */ > +/* { dg-options "-O3 -fno-delete-null-pointer-checks -fdump-tree-optimized > -fno-allocation-dce" } */ > > #include <new> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dce-3.c > b/gcc/testsuite/gcc.dg/tree-ssa/dce-3.c > new file mode 100644 > index 00000000000..5b53e8f9767 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/dce-3.c > @@ -0,0 +1,34 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-cddce1" } */ > +void > +test (int s) > +{ > + int *mem = (int *) __builtin_malloc (s * sizeof (int)); > + if (mem) > + { > + for (int i = 0; i < s; i++) > + mem[i] = i; > + __builtin_free (mem); > + } > +} > + > +void > +test2 (int s) > +{ > + void *mem = __builtin_malloc (s); > + if (!mem) > + __builtin_abort (); > + __builtin_free (mem); > +} > +void > +test3 (char *s) > +{ > + char *c = __builtin_strdup (s); > + if (!c) > + __builtin_abort (); > + __builtin_free (c); > +} > +/* { dg-final { scan-tree-dump-not "if" "cddce1"} } */ > +/* { dg-final { scan-tree-dump-not "alloc" "cddce1"} } */ > +/* { dg-final { scan-tree-dump-not "free" "cddce1"} } */ > +/* { dg-final { scan-tree-dump-not "strdup" "cddce1"} } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr19831-3.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr19831-3.c > index f5cb72daa33..e6ae6582ab2 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/pr19831-3.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr19831-3.c > @@ -29,10 +29,5 @@ void test6(void) > Assume p was non-NULL for test2. > For test5, it doesn't matter if p is NULL or non-NULL. */ > > -/* { dg-final { scan-tree-dump-times "free" 0 "optimized" { xfail *-*-* } } > } */ > -/* { dg-final { scan-tree-dump-times "malloc" 0 "optimized" { xfail *-*-* } > } } */ > - > -/* But make sure we don't partially optimize for now. */ > - > -/* { dg-final { scan-tree-dump-times "free" 3 "optimized" } } */ > -/* { dg-final { scan-tree-dump-times "malloc" 3 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "free" 0 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "malloc" 0 "optimized" } } */ > diff --git a/gcc/testsuite/gfortran.dg/pr68078.f90 > b/gcc/testsuite/gfortran.dg/pr68078.f90 > index ebe26d55d2b..b71b0fa4e33 100644 > --- a/gcc/testsuite/gfortran.dg/pr68078.f90 > +++ b/gcc/testsuite/gfortran.dg/pr68078.f90 > @@ -1,4 +1,6 @@ > ! { dg-do run { target i?86-*-linux* x86_64-*-linux* } } > +! disable DCE so the allocations are not optimized away > +! { dg-additional-options "-fno-tree-dce" } > ! { dg-additional-sources set_vm_limit.c } > ! > ! This test calls set_vm_limit to set an artificially low address space > diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc > index c44eff35b84..aa77c7367bf 100644 > --- a/gcc/tree-ssa-dce.cc > +++ b/gcc/tree-ssa-dce.cc > @@ -240,6 +240,55 @@ mark_operand_necessary (tree op) > worklist.safe_push (stmt); > } > > +/* Return true if STMT is a call to allocation function that can be > + optimized out if the memory block is never used and only freed. */ > + > +static bool > +is_removable_allocation_p (gcall *stmt) > +{ > + tree callee = gimple_call_fndecl (stmt); > + if (callee != NULL_TREE > + && fndecl_built_in_p (callee, BUILT_IN_NORMAL)) > + switch (DECL_FUNCTION_CODE (callee)) > + { > + case BUILT_IN_MALLOC: > + case BUILT_IN_ALIGNED_ALLOC: > + case BUILT_IN_CALLOC: > + CASE_BUILT_IN_ALLOCA: > + case BUILT_IN_STRDUP: > + case BUILT_IN_STRNDUP: > + case BUILT_IN_GOMP_ALLOC: > + return true; > + > + default:; > + } > + > + if (callee != NULL_TREE > + && flag_allocation_dce > + && gimple_call_from_new_or_delete (stmt) > + && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)) > + return true; > + return false; > +} > + > +/* Return true if STMT is a conditional > + if (ptr != NULL) > + where ptr was returned by a removable allocation function. */ > + > +static bool > +checks_return_value_of_removable_allocation_p (gimple *stmt) > +{ > + gcall *def_stmt; > + return gimple_code (stmt) == GIMPLE_COND > + && (gimple_cond_code (stmt) == EQ_EXPR > + || gimple_cond_code (stmt) == NE_EXPR) > + && integer_zerop (gimple_cond_rhs (stmt)) > + && TREE_CODE (gimple_cond_lhs (stmt)) == SSA_NAME > + && (def_stmt = dyn_cast <gcall *> > + (SSA_NAME_DEF_STMT (gimple_cond_lhs (stmt)))) > + && is_removable_allocation_p (def_stmt); > +} > + > > /* Mark STMT as necessary if it obviously is. Add it to the worklist if > it can make other statements necessary. > @@ -271,38 +320,23 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool > aggressive) > > case GIMPLE_CALL: > { > + gcall *call = as_a <gcall *> (stmt); > + > /* Never elide a noreturn call we pruned control-flow for. */ > - if ((gimple_call_flags (stmt) & ECF_NORETURN) > - && gimple_call_ctrl_altering_p (stmt)) > + if ((gimple_call_flags (call) & ECF_NORETURN) > + && gimple_call_ctrl_altering_p (call)) > { > - mark_stmt_necessary (stmt, true); > + mark_stmt_necessary (call, true); > return; > } > > - tree callee = gimple_call_fndecl (stmt); > - if (callee != NULL_TREE > - && fndecl_built_in_p (callee, BUILT_IN_NORMAL)) > - switch (DECL_FUNCTION_CODE (callee)) > - { > - case BUILT_IN_MALLOC: > - case BUILT_IN_ALIGNED_ALLOC: > - case BUILT_IN_CALLOC: > - CASE_BUILT_IN_ALLOCA: > - case BUILT_IN_STRDUP: > - case BUILT_IN_STRNDUP: > - case BUILT_IN_GOMP_ALLOC: > - return; > - > - default:; > - } > > - if (callee != NULL_TREE > - && flag_allocation_dce > - && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)) > + if (is_removable_allocation_p (call)) > return; > > + > /* For __cxa_atexit calls, don't mark as necessary right away. */ > - if (is_removable_cxa_atexit_call (stmt)) > + if (is_removable_cxa_atexit_call (call)) > return; > > /* IFN_GOACC_LOOP calls are necessary in that they are used to > @@ -311,9 +345,9 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool > aggressive) > survive from aggressive loop removal for it has loop exit and > is assumed to be finite. Therefore, we need to explicitly mark > these calls. (An example is libgomp.oacc-c-c++-common/pr84955.c) */ > - if (gimple_call_internal_p (stmt, IFN_GOACC_LOOP)) > + if (gimple_call_internal_p (call, IFN_GOACC_LOOP)) > { > - mark_stmt_necessary (stmt, true); > + mark_stmt_necessary (call, true); > return; > } > break; > @@ -667,6 +701,8 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref > ATTRIBUTE_UNUSED, > case BUILT_IN_ALIGNED_ALLOC: > case BUILT_IN_CALLOC: > CASE_BUILT_IN_ALLOCA: > + case BUILT_IN_STRDUP: > + case BUILT_IN_STRNDUP: > case BUILT_IN_FREE: > case BUILT_IN_GOMP_ALLOC: > case BUILT_IN_GOMP_FREE: > @@ -891,19 +927,11 @@ propagate_necessity (bool aggressive) > { > tree ptr = gimple_call_arg (stmt, 0); > gcall *def_stmt; > - tree def_callee; > /* If the pointer we free is defined by an allocation > function do not add the call to the worklist. */ > if (TREE_CODE (ptr) == SSA_NAME > && (def_stmt = dyn_cast <gcall *> (SSA_NAME_DEF_STMT (ptr))) > - && (def_callee = gimple_call_fndecl (def_stmt)) > - && ((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_GOMP_ALLOC)) > - || (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee) > - && gimple_call_from_new_or_delete (def_stmt)))) > + && is_removable_allocation_p (def_stmt)) > { > if (is_delete_operator > && !valid_new_delete_pair_p (def_stmt, stmt)) > @@ -925,6 +953,9 @@ propagate_necessity (bool aggressive) > } > } > > + if (checks_return_value_of_removable_allocation_p (stmt)) > + continue; > + > FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE) > mark_operand_necessary (use); > > @@ -1379,7 +1410,6 @@ eliminate_unnecessary_stmts (bool aggressive) > basic_block bb; > gimple_stmt_iterator gsi, psi; > gimple *stmt; > - tree call; > auto_vec<edge> to_remove_edges; > > if (dump_file && (dump_flags & TDF_DETAILS)) > @@ -1448,6 +1478,23 @@ eliminate_unnecessary_stmts (bool aggressive) > gimple_set_plf (stmt, STMT_NECESSARY, false); > } > } > + /* Conditional checking that return value of allocation is non-NULL > + can be turned to constant if the allocation itself > + is unnecesary. */ > + if (gimple_plf (stmt, STMT_NECESSARY) > + && gimple_code (stmt) == GIMPLE_COND > + && TREE_CODE (gimple_cond_lhs (stmt)) == SSA_NAME) > + { > + gimple *def_stmt = SSA_NAME_DEF_STMT (gimple_cond_lhs (stmt)); > + if (!gimple_nop_p (def_stmt) > + && !gimple_plf (def_stmt, STMT_NECESSARY)) > + { > + gcc_checking_assert > + (checks_return_value_of_removable_allocation_p > (stmt)); > + gimple_cond_set_lhs (as_a <gcond *>(stmt), > integer_one_node); > + update_stmt (stmt); > + } > + } > > /* If GSI is not necessary then remove it. */ > if (!gimple_plf (stmt, STMT_NECESSARY)) > @@ -1482,11 +1529,11 @@ eliminate_unnecessary_stmts (bool aggressive) > remove_dead_stmt (&gsi, bb, to_remove_edges); > continue; > } > - else if (is_gimple_call (stmt)) > + else if (gcall *call_stmt = dyn_cast <gcall *> (stmt)) > { > - tree name = gimple_call_lhs (stmt); > + tree name = gimple_call_lhs (call_stmt); > > - notice_special_calls (as_a <gcall *> (stmt)); > + notice_special_calls (call_stmt); > > /* When LHS of var = call (); is dead, simplify it into > call (); saving one operand. */ > @@ -1496,36 +1543,30 @@ eliminate_unnecessary_stmts (bool aggressive) > /* Avoid doing so for allocation calls which we > did not mark as necessary, it will confuse the > special logic we apply to malloc/free pair removal. */ > - && (!(call = gimple_call_fndecl (stmt)) > - || ((DECL_BUILT_IN_CLASS (call) != BUILT_IN_NORMAL > - || (DECL_FUNCTION_CODE (call) != > BUILT_IN_ALIGNED_ALLOC > - && DECL_FUNCTION_CODE (call) != BUILT_IN_MALLOC > - && DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC > - && !ALLOCA_FUNCTION_CODE_P > - (DECL_FUNCTION_CODE (call)))) > - && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (call)))) > + && !is_removable_allocation_p (call_stmt)) > { > something_changed = true; > if (dump_file && (dump_flags & TDF_DETAILS)) > { > fprintf (dump_file, "Deleting LHS of call: "); > - print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM); > + print_gimple_stmt (dump_file, call_stmt, 0, TDF_SLIM); > fprintf (dump_file, "\n"); > } > > - gimple_call_set_lhs (stmt, NULL_TREE); > - maybe_clean_or_replace_eh_stmt (stmt, stmt); > - update_stmt (stmt); > + gimple_call_set_lhs (call_stmt, NULL_TREE); > + maybe_clean_or_replace_eh_stmt (call_stmt, call_stmt); > + update_stmt (call_stmt); > release_ssa_name (name); > > /* GOMP_SIMD_LANE (unless three argument) or ASAN_POISON > without lhs is not needed. */ > - if (gimple_call_internal_p (stmt)) > - switch (gimple_call_internal_fn (stmt)) > + if (gimple_call_internal_p (call_stmt)) > + switch (gimple_call_internal_fn (call_stmt)) > { > case IFN_GOMP_SIMD_LANE: > - if (gimple_call_num_args (stmt) >= 3 > - && !integer_nonzerop (gimple_call_arg (stmt, 2))) > + if (gimple_call_num_args (call_stmt) >= 3 > + && !integer_nonzerop > + (gimple_call_arg (call_stmt, 2))) > break; > /* FALLTHRU */ > case IFN_ASAN_POISON: > @@ -1535,8 +1576,8 @@ eliminate_unnecessary_stmts (bool aggressive) > break; > } > } > - else if (gimple_call_internal_p (stmt)) > - switch (gimple_call_internal_fn (stmt)) > + else if (gimple_call_internal_p (call_stmt)) > + switch (gimple_call_internal_fn (call_stmt)) > { > case IFN_ADD_OVERFLOW: > maybe_optimize_arith_overflow (&gsi, PLUS_EXPR); > @@ -1548,11 +1589,11 @@ eliminate_unnecessary_stmts (bool aggressive) > maybe_optimize_arith_overflow (&gsi, MULT_EXPR); > break; > case IFN_UADDC: > - if (integer_zerop (gimple_call_arg (stmt, 2))) > + if (integer_zerop (gimple_call_arg (call_stmt, 2))) > maybe_optimize_arith_overflow (&gsi, PLUS_EXPR); > break; > case IFN_USUBC: > - if (integer_zerop (gimple_call_arg (stmt, 2))) > + if (integer_zerop (gimple_call_arg (call_stmt, 2))) > maybe_optimize_arith_overflow (&gsi, MINUS_EXPR); > break; > default:
