Thanks for all the reviews! I’ve revised the patch, the operator_delete_flag is now stored in tree_decl_with_vis (there already seem to be some FUNCTION_DECL-flags in there). I’ve also added the option -fallocation-dce to disable this optimization. It bootstraps and no regressions on aarch64 and x86_64.
The problem with this patch is what Marc noticed: it omits too many allocations. The C++ standard seems to only allow to omit "replaceable global allocation functions (18.6.1.1, 18.6.1.2)”. So e.g. no class-specific or user-defined allocations. I am not sure what’s the best way to implement this. Just checking the function declarations might not be enough and seems more like a hack. The better way seems to introduce a __builtin_operator_new like Marc mentioned. In which way would you implement this? Could you please give me some pointers here to look at? Thanks, Dominik
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index d95a2b6ea4f..4ed8c2d191a 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -2454,6 +2454,7 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype) TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl); DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl); DECL_IS_OPERATOR_NEW (newdecl) |= DECL_IS_OPERATOR_NEW (olddecl); + DECL_IS_OPERATOR_DELETE (newdecl) |= DECL_IS_OPERATOR_DELETE (olddecl); TREE_READONLY (newdecl) |= TREE_READONLY (olddecl); DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl); DECL_IS_NOVOPS (newdecl) |= DECL_IS_NOVOPS (olddecl); diff --git a/gcc/common.opt b/gcc/common.opt index f8f2ed3db8a..526c7df63d7 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2119,6 +2119,10 @@ starts and when the destructor finishes. flifetime-dse= Common Joined RejectNegative UInteger Var(flag_lifetime_dse) Optimization IntegerRange(0, 2) +fallocation-dce +Common Report Var(flag_allocation_dce) Init(1) Optimization +Tell DCE to remove unused C++ allocations. + flive-range-shrinkage Common Report Var(flag_live_range_shrinkage) Init(0) Optimization Relief of register pressure through live range shrinkage. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 54e06568e46..d1c77b1d670 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -4182,8 +4182,11 @@ cxx_init_decl_processing (void) opnew = push_cp_library_fn (VEC_NEW_EXPR, newtype, 0); DECL_IS_MALLOC (opnew) = 1; DECL_IS_OPERATOR_NEW (opnew) = 1; - push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); - push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + tree opdel = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; + opdel = push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; + if (flag_sized_deallocation) { /* Also push the sized deallocation variants: @@ -4195,8 +4198,10 @@ cxx_init_decl_processing (void) deltype = cp_build_type_attribute_variant (void_ftype_ptr_size, extvisattr); deltype = build_exception_variant (deltype, empty_except_spec); - push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + opdel = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; } if (aligned_new_threshold) @@ -4224,8 +4229,10 @@ cxx_init_decl_processing (void) align_type_node, NULL_TREE); deltype = cp_build_type_attribute_variant (deltype, extvisattr); deltype = build_exception_variant (deltype, empty_except_spec); - push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); - push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + opdel = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; + opdel = push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; if (flag_sized_deallocation) { @@ -4235,8 +4242,10 @@ cxx_init_decl_processing (void) NULL_TREE); deltype = cp_build_type_attribute_variant (deltype, extvisattr); deltype = build_exception_variant (deltype, empty_except_spec); - push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); - push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + opdel = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; + opdel = push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_OPERATOR_DELETE (opdel) = 1; } } diff --git a/gcc/gimple.c b/gcc/gimple.c index c986a732004..b4cce195e57 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -2543,6 +2543,30 @@ gimple_builtin_call_types_compatible_p (const gimple *stmt, tree fndecl) return true; } +/* Return true when STMT is operator new call. */ + +bool +gimple_call_operator_new_p (const gcall *stmt) +{ + tree fndecl; + + if ((fndecl = gimple_call_fndecl (stmt)) != NULL_TREE) + return DECL_IS_OPERATOR_NEW (fndecl); + return false; +} + +/* Return true when STMT is operator delete call. */ + +bool +gimple_call_operator_delete_p (const gcall *stmt) +{ + tree fndecl; + + if ((fndecl = gimple_call_fndecl (stmt)) != NULL_TREE) + return DECL_IS_OPERATOR_DELETE (fndecl); + return false; +} + /* Return true when STMT is builtins call. */ bool diff --git a/gcc/gimple.h b/gcc/gimple.h index 334def89398..01d37227f09 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -1514,6 +1514,8 @@ extern alias_set_type gimple_get_alias_set (tree); extern bool gimple_ior_addresses_taken (bitmap, gimple *); extern bool gimple_builtin_call_types_compatible_p (const gimple *, tree); extern combined_fn gimple_call_combined_fn (const gimple *); +extern bool gimple_call_operator_new_p (const gcall *); +extern bool gimple_call_operator_delete_p (const gcall *); extern bool gimple_call_builtin_p (const gimple *); extern bool gimple_call_builtin_p (const gimple *, enum built_in_class); extern bool gimple_call_builtin_p (const gimple *, enum built_in_function); diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C b/gcc/testsuite/g++.dg/cpp1y/new1.C new file mode 100644 index 00000000000..b966cf5d7b7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C @@ -0,0 +1,65 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-cddce-details" } */ + +#include <stdlib.h> + +void +new_without_use() { + int *x = new int; +} + +void +new_array_without_use() { + int *x = new int[5]; +} + +void +new_primitive() { + int *x = new int; + delete x; +} + +void +new_array() { + int *x = new int[10]; + delete [] x; +} + +void +new_primitive_store() { + int *x = new int; + *x = 10; + delete x; +} + +void +new_primitive_load() { + int *x = new int; + int tmp = *x; + delete x; +} + +int +new_primitive_load_with_use() { + int *x = new int; + int tmp = *x; + delete x; + return tmp; +} + +void +new_array_store() { + int *x = new int[10]; + x[4] = 10; + delete [] x; +} + +void +new_array_load() { + int *x = new int[10]; + int tmp = x[4]; + delete [] x; +} + +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 4 "cddce1"} } */ +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 6 "cddce1"} } */ diff --git a/gcc/tree-core.h b/gcc/tree-core.h index aa54221253c..f3ee0dea98b 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1723,7 +1723,9 @@ struct GTY(()) tree_decl_with_vis { unsigned final : 1; /* Belong to FUNCTION_DECL exclusively. */ unsigned regdecl_flag : 1; - /* 14 unused bits. */ + /* Belong to FUNCTION_DECL exclusively. */ + unsigned operator_delete_flag : 1; + /* 13 unused bits. */ }; struct GTY(()) tree_var_decl { diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index a5f0edf7893..e9086911d12 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -238,6 +238,12 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive) default:; } + + if (callee != NULL_TREE + && flag_allocation_dce + && DECL_IS_OPERATOR_NEW (callee)) + return; + /* Most, but not all function calls are required. Function calls that produce no result and have no side effects (i.e. const pure functions) are unnecessary. */ @@ -581,6 +587,11 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED, default:; } + + if (callee != NULL_TREE + && (DECL_IS_OPERATOR_NEW (callee) + || DECL_IS_OPERATOR_DELETE (callee))) + return false; } if (! gimple_clobber_p (def_stmt)) @@ -767,20 +778,24 @@ propagate_necessity (bool aggressive) /* If this is a call to free which is directly fed by an allocation function do not mark that necessary through processing the argument. */ - if (gimple_call_builtin_p (stmt, BUILT_IN_FREE)) + if (gimple_call_builtin_p (stmt, BUILT_IN_FREE) + || (is_gimple_call (stmt) + && gimple_call_operator_delete_p (as_a <gcall *> (stmt)))) { tree ptr = gimple_call_arg (stmt, 0); gimple *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 && is_gimple_call (def_stmt = 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_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_IS_OPERATOR_NEW (def_callee))) { gimple *bounds_def_stmt; tree bounds; @@ -794,7 +809,24 @@ propagate_necessity (bool aggressive) && chkp_gimple_call_builtin_p (bounds_def_stmt, BUILT_IN_CHKP_BNDRET) && gimple_call_arg (bounds_def_stmt, 0) == ptr)) - continue; + { + /* For operator delete [] we need to keep size operand + alive. Otherwise this operand isn't available anymore + when we finally decide that this stmt is necessary in + eliminate_unnecessary_stmts. If it should really be + unnecessary, a later pass can clean this up. */ + if (gimple_call_operator_delete_p (as_a <gcall *> (stmt))) + { + for (unsigned i=1; i < gimple_call_num_args (stmt); ++i) + { + tree arg = gimple_call_arg (stmt, i); + if (TREE_CODE (arg) == SSA_NAME) + mark_operand_necessary (arg); + } + } + + continue; + } } } @@ -849,6 +881,11 @@ propagate_necessity (bool aggressive) || DECL_FUNCTION_CODE (callee) == BUILT_IN_ASSUME_ALIGNED)) continue; + if (callee != NULL_TREE + && (DECL_IS_OPERATOR_NEW (callee) + || DECL_IS_OPERATOR_DELETE (callee))) + continue; + /* Calls implicitly load from memory, their arguments in addition may explicitly perform memory loads. */ mark_all_reaching_defs_necessary (stmt); @@ -1269,7 +1306,9 @@ eliminate_unnecessary_stmts (void) defining statement of its argument is not necessary (and thus is getting removed). */ if (gimple_plf (stmt, STMT_NECESSARY) - && gimple_call_builtin_p (stmt, BUILT_IN_FREE)) + && (gimple_call_builtin_p (stmt, BUILT_IN_FREE) + || (is_gimple_call (stmt) + && gimple_call_operator_delete_p (as_a <gcall *> (stmt))))) { tree ptr = gimple_call_arg (stmt, 0); if (TREE_CODE (ptr) == SSA_NAME) diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c index baf0c5bf837..eb8a2aa1a93 100644 --- a/gcc/tree-streamer-in.c +++ b/gcc/tree-streamer-in.c @@ -326,6 +326,7 @@ unpack_ts_function_decl_value_fields (struct bitpack_d *bp, tree expr) DECL_IS_RETURNS_TWICE (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_IS_MALLOC (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_IS_OPERATOR_NEW (expr) = (unsigned) bp_unpack_value (bp, 1); + DECL_IS_OPERATOR_DELETE (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_DECLARED_INLINE_P (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_STATIC_CHAIN (expr) = (unsigned) bp_unpack_value (bp, 1); DECL_NO_INLINE_WARNING_P (expr) = (unsigned) bp_unpack_value (bp, 1); diff --git a/gcc/tree-streamer-out.c b/gcc/tree-streamer-out.c index 7f52d455f5e..a63d16deb23 100644 --- a/gcc/tree-streamer-out.c +++ b/gcc/tree-streamer-out.c @@ -287,6 +287,7 @@ pack_ts_function_decl_value_fields (struct bitpack_d *bp, tree expr) bp_pack_value (bp, DECL_IS_RETURNS_TWICE (expr), 1); bp_pack_value (bp, DECL_IS_MALLOC (expr), 1); bp_pack_value (bp, DECL_IS_OPERATOR_NEW (expr), 1); + bp_pack_value (bp, DECL_IS_OPERATOR_DELETE (expr), 1); bp_pack_value (bp, DECL_DECLARED_INLINE_P (expr), 1); bp_pack_value (bp, DECL_STATIC_CHAIN (expr), 1); bp_pack_value (bp, DECL_NO_INLINE_WARNING_P (expr), 1); diff --git a/gcc/tree.h b/gcc/tree.h index 0ec092aa812..98f6ea0f6d4 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2866,6 +2866,11 @@ extern void decl_fini_priority_insert (tree, priority_type); #define DECL_IS_OPERATOR_NEW(NODE) \ (FUNCTION_DECL_CHECK (NODE)->function_decl.operator_new_flag) +/* Nonzero in a FUNCTION_DECL means this function should be treated as + C++ operator delete. */ +#define DECL_IS_OPERATOR_DELETE(NODE) \ + (FUNCTION_DECL_CHECK (NODE)->decl_with_vis.operator_delete_flag) + /* Nonzero in a FUNCTION_DECL means this function may return more than once. */ #define DECL_IS_RETURNS_TWICE(NODE) \ diff --git a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc index bc36ed595e0..1df4b71b598 100644 --- a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc +++ b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc @@ -15,6 +15,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. +// { dg-options "-fno-allocation-dce" } + // 20.4.1.1 allocator members #include <cstdlib> diff --git a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_new.cc b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_new.cc index 073d588f3b9..c7d745249f5 100644 --- a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_new.cc +++ b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_new.cc @@ -15,6 +15,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. +// { dg-options "-fno-allocation-dce" } + // 20.4.1.1 allocator members #include <cstdlib> diff --git a/libstdc++-v3/testsuite/ext/new_allocator/check_delete.cc b/libstdc++-v3/testsuite/ext/new_allocator/check_delete.cc index ce6c1da3e27..aa098ad2c65 100644 --- a/libstdc++-v3/testsuite/ext/new_allocator/check_delete.cc +++ b/libstdc++-v3/testsuite/ext/new_allocator/check_delete.cc @@ -17,6 +17,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. +// { dg-options "-fno-allocation-dce" } + // 20.4.1.1 allocator members #include <cstdlib> diff --git a/libstdc++-v3/testsuite/ext/new_allocator/check_new.cc b/libstdc++-v3/testsuite/ext/new_allocator/check_new.cc index d410f5f7feb..21ad642a3fe 100644 --- a/libstdc++-v3/testsuite/ext/new_allocator/check_new.cc +++ b/libstdc++-v3/testsuite/ext/new_allocator/check_new.cc @@ -17,6 +17,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. +// { dg-options "-fno-allocation-dce" } + // 20.4.1.1 allocator members #include <cstdlib>
> On 22 Nov 2017, at 11:37, Jakub Jelinek <ja...@redhat.com> wrote: > > On Wed, Nov 22, 2017 at 10:30:29AM +0100, Richard Biener wrote: >> --- a/gcc/tree-core.h >> +++ b/gcc/tree-core.h >> @@ -1787,7 +1787,9 @@ struct GTY(()) tree_function_decl { >> unsigned has_debug_args_flag : 1; >> unsigned tm_clone_flag : 1; >> unsigned versioned_function : 1; >> - /* No bits left. */ >> + >> + unsigned operator_delete_flag : 1; >> + /* 31 bits left. */ >> >> while it looks bad reality is that on 64bit pointer hosts we had 32 bits >> left. > > But we can just add it to say tree_decl_common which has 14 spare bits or > tree_decl_with_vis which has another 14 spare bits. By just noting the flag > applies only to FUNCTION_DECLs and enforcing it in the tree.h macros, > it will be easy to reuse that bit for something different for trees other > than FUNCTION_DECL. > > Jakub
signature.asc
Description: Message signed with OpenPGP using GPGMail