Hi, all! Currently GCC can optimize away the following dead store:
void test(char *x) { *x = 1; free(x); } but not this one (Clang handles both cases): void test(char *x) { *x = 1; delete x; } The attached patch fixes this by introducing a new __attribute__((free)). I first tried to add new built-ins for each version of operator delete (there are four of them), but it looked a little clumsy, and would require some special handling for warning about taking address of built-in function. Is such approach (i.e. adding a new attribute) OK? Bootstrapped and regtested on x86_64-pc-linux-gnu. -- Regards, Mikhail Maltsev gcc/c/ChangeLog: 2016-04-16 Mikhail Maltsev <malts...@gmail.com> * c-decl.c (merge_decls): Handle free_flag. gcc/ChangeLog: 2016-04-16 Mikhail Maltsev <malts...@gmail.com> * builtin-attrs.def: Add attribute free. * builtins.def (free): Add attribute free. * doc/extend.texi: Document attribute free. * gtm-builtins.def (_ITM_free): Add attribute free. * tree-core.h (struct tree_function_decl): Add free_flag. * tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Handle free_flag. (call_may_clobber_ref_p_1): Likewise. (stmt_kills_ref_p): Likewise. * tree-streamer-in.c (unpack_ts_function_decl_value_fields): Likewise. * tree-streamer-out.c (pack_ts_function_decl_value_fields): Likewise. * tree.h (DECL_IS_FREE): New accessor macro. gcc/testsuite/ChangeLog: 2016-04-16 Mikhail Maltsev <malts...@gmail.com> * g++.dg/opt/op-delete-dse.C: New test. * gcc.dg/attr-free.c: New test. gcc/c-family/ChangeLog: 2016-04-16 Mikhail Maltsev <malts...@gmail.com> * c-common.c (handle_free_attribute): New function. gcc/cp/ChangeLog: 2016-04-16 Mikhail Maltsev <malts...@gmail.com> * decl.c (cxx_init_decl_processing): Set flag_free for operator delete.
diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def index 089817a..ddaf3e6 100644 --- a/gcc/builtin-attrs.def +++ b/gcc/builtin-attrs.def @@ -88,6 +88,7 @@ DEF_ATTR_IDENT (ATTR_CONST, "const") DEF_ATTR_IDENT (ATTR_FORMAT, "format") DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg") DEF_ATTR_IDENT (ATTR_MALLOC, "malloc") +DEF_ATTR_IDENT (ATTR_FREE, "free") DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull") DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn") DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow") @@ -141,6 +142,10 @@ DEF_ATTR_TREE_LIST (ATTR_MALLOC_NOTHROW_LIST, ATTR_MALLOC, \ ATTR_NULL, ATTR_NOTHROW_LIST) DEF_ATTR_TREE_LIST (ATTR_MALLOC_NOTHROW_LEAF_LIST, ATTR_MALLOC, \ ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) +DEF_ATTR_TREE_LIST (ATTR_FREE_NOTHROW_LIST, ATTR_FREE, \ + ATTR_NULL, ATTR_NOTHROW_LIST) +DEF_ATTR_TREE_LIST (ATTR_FREE_NOTHROW_LEAF_LIST, ATTR_FREE, \ + ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) DEF_ATTR_TREE_LIST (ATTR_SENTINEL_NOTHROW_LIST, ATTR_SENTINEL, \ ATTR_NULL, ATTR_NOTHROW_LIST) DEF_ATTR_TREE_LIST (ATTR_SENTINEL_NOTHROW_LEAF_LIST, ATTR_SENTINEL, \ @@ -269,8 +274,10 @@ DEF_ATTR_TREE_LIST (ATTR_TM_NOTHROW_RT_LIST, DEF_ATTR_TREE_LIST (ATTR_TMPURE_MALLOC_NOTHROW_LIST, ATTR_TM_TMPURE, ATTR_NULL, ATTR_MALLOC_NOTHROW_LIST) /* Same attributes used for BUILT_IN_FREE except with TM_PURE thrown in. */ -DEF_ATTR_TREE_LIST (ATTR_TMPURE_NOTHROW_LIST, - ATTR_TM_TMPURE, ATTR_NULL, ATTR_NOTHROW_LIST) +DEF_ATTR_TREE_LIST (ATTR_TMPURE_FREE_NOTHROW_LIST, + ATTR_TM_TMPURE, ATTR_NULL, ATTR_FREE_NOTHROW_LIST) +DEF_ATTR_TREE_LIST (ATTR_TMPURE_FREE_NOTHROW_LEAF_LIST, + ATTR_TM_TMPURE, ATTR_NULL, ATTR_FREE_NOTHROW_LEAF_LIST) DEF_ATTR_TREE_LIST (ATTR_TMPURE_NOTHROW_LEAF_LIST, ATTR_TM_TMPURE, ATTR_NULL, ATTR_NOTHROW_LEAF_LIST) diff --git a/gcc/builtins.def b/gcc/builtins.def index 2fc7f65..e3d1614 100644 --- a/gcc/builtins.def +++ b/gcc/builtins.def @@ -781,7 +781,7 @@ DEF_EXT_LIB_BUILTIN (BUILT_IN_FFSLL, "ffsll", BT_FN_INT_LONGLONG, ATTR_CONST_ DEF_EXT_LIB_BUILTIN (BUILT_IN_FORK, "fork", BT_FN_PID, ATTR_NOTHROW_LIST) DEF_GCC_BUILTIN (BUILT_IN_FRAME_ADDRESS, "frame_address", BT_FN_PTR_UINT, ATTR_NULL) /* [trans-mem]: Adjust BUILT_IN_TM_FREE if BUILT_IN_FREE is changed. */ -DEF_LIB_BUILTIN (BUILT_IN_FREE, "free", BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST) +DEF_LIB_BUILTIN (BUILT_IN_FREE, "free", BT_FN_VOID_PTR, ATTR_FREE_NOTHROW_LEAF_LIST) DEF_GCC_BUILTIN (BUILT_IN_FROB_RETURN_ADDR, "frob_return_addr", BT_FN_PTR_PTR, ATTR_NULL) DEF_EXT_LIB_BUILTIN (BUILT_IN_GETTEXT, "gettext", BT_FN_STRING_CONST_STRING, ATTR_FORMAT_ARG_1) DEF_C99_BUILTIN (BUILT_IN_IMAXABS, "imaxabs", BT_FN_INTMAX_INTMAX, ATTR_CONST_NOTHROW_LEAF_LIST) diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 30c815d..3840675 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -355,6 +355,7 @@ static tree handle_tls_model_attribute (tree *, tree, tree, int, static tree handle_no_instrument_function_attribute (tree *, tree, tree, int, bool *); static tree handle_malloc_attribute (tree *, tree, tree, int, bool *); +static tree handle_free_attribute (tree *, tree, tree, int, bool *); static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *); static tree handle_no_limit_stack_attribute (tree *, tree, tree, int, bool *); @@ -720,6 +721,8 @@ const struct attribute_spec c_common_attribute_table[] = false }, { "malloc", 0, 0, true, false, false, handle_malloc_attribute, false }, + { "free", 0, 0, true, false, false, + handle_free_attribute, false }, { "returns_twice", 0, 0, true, false, false, handle_returns_twice_attribute, false }, { "no_stack_limit", 0, 0, true, false, false, @@ -8315,6 +8318,27 @@ handle_malloc_attribute (tree *node, tree name, tree ARG_UNUSED (args), return NULL_TREE; } +/* Handle a "free" attribute; arguments as in struct attribute_spec.handler. */ + +static tree +handle_free_attribute (tree *node, tree name, tree /*args*/, int /*flags*/, + bool *no_add_attrs) +{ + tree decl = *node; + if (TREE_CODE (decl) == FUNCTION_DECL + && type_num_arguments (TREE_TYPE (decl)) != 0 + && POINTER_TYPE_P (TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (decl))))) + DECL_IS_FREE (decl) = 1; + else + { + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes, + "%qE attribute ignored", name); + *no_add_attrs = true; + } + + return NULL_TREE; +} + /* Handle a "alloc_size" attribute; arguments as in struct attribute_spec.handler. */ diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 0dd2121..2941211 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -2478,6 +2478,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_FREE (newdecl) |= DECL_IS_FREE (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/cp/decl.c b/gcc/cp/decl.c index 9260f4c..b9187bf 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -4123,8 +4123,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 opdelete = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_FREE (opdelete) = true; + opdelete = push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_FREE (opdelete) = true; if (flag_sized_deallocation) { /* Also push the sized deallocation variants: @@ -4136,8 +4139,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); - push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + opdelete = push_cp_library_fn (DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_FREE (opdelete) = true; + opdelete = push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW); + DECL_IS_FREE (opdelete) = true; } nullptr_type_node = make_node (NULLPTR_TYPE); diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 6e27029..4d610ea 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2821,6 +2821,17 @@ a pointer to uninitialized or zeroed-out storage. However, functions like @code{realloc} do not have this property, as they can return a pointer to storage containing pointers. +@item free +@cindex @code{free} function attribute +@cindex functions that behave like free +This tells the compiler that a function is @code{free}-like, i.e., that it +does not access the storage addressed by the pointer @var{P} passed to the +function. Moreover, accessing the storage after the function returns invokes +undefined behavior. + +Using this attribute can expose more opportunities to dead store elimination +optimization. + @item no_icf @cindex @code{no_icf} function attribute This function attribute prevents a functions from being merged with another diff --git a/gcc/gtm-builtins.def b/gcc/gtm-builtins.def index 6d5cfb9..556391b 100644 --- a/gcc/gtm-builtins.def +++ b/gcc/gtm-builtins.def @@ -32,7 +32,7 @@ DEF_TM_BUILTIN (BUILT_IN_TM_MALLOC, "_ITM_malloc", DEF_TM_BUILTIN (BUILT_IN_TM_CALLOC, "_ITM_calloc", BT_FN_PTR_SIZE_SIZE, ATTR_TMPURE_MALLOC_NOTHROW_LIST) DEF_TM_BUILTIN (BUILT_IN_TM_FREE, "_ITM_free", - BT_FN_VOID_PTR, ATTR_TMPURE_NOTHROW_LEAF_LIST) + BT_FN_VOID_PTR, ATTR_TMPURE_FREE_NOTHROW_LEAF_LIST) /* Logging builtins. */ DEF_TM_BUILTIN (BUILT_IN_TM_LOG_1, "_ITM_LU1", diff --git a/gcc/testsuite/g++.dg/opt/op-delete-dse.C b/gcc/testsuite/g++.dg/opt/op-delete-dse.C new file mode 100644 index 0000000..4df869f --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/op-delete-dse.C @@ -0,0 +1,14 @@ +// { dg-do compile } +// { dg-options "-O -fdump-tree-dse1-details" } +// { dg-final { scan-tree-dump-times "Deleted dead call" 1 "dse1" } } + +void use (void *); + +void +test_delete_pod () +{ + int *data = new int; + use (data); + __builtin_memset (data, 0, sizeof (int)); + delete data; +} diff --git a/gcc/testsuite/gcc.dg/attr-free.c b/gcc/testsuite/gcc.dg/attr-free.c new file mode 100644 index 0000000..6aa9153 --- /dev/null +++ b/gcc/testsuite/gcc.dg/attr-free.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ + +void *malloc (__SIZE_TYPE__); +void custom_free (void *) __attribute__((free)); + +void +test (void) +{ + char *data = (char *) malloc (1); + data[0] = 42; + custom_free (data); +} + +/* { dg-final { scan-assembler-not "42" } } */ diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 41c1a9b..e4d9921 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1665,6 +1665,8 @@ struct GTY(()) tree_function_decl { /* Index within a virtual table. */ tree vindex; + /* Bitfields: word 1. */ + /* In a FUNCTION_DECL for which DECL_BUILT_IN holds, this is DECL_FUNCTION_CODE. Otherwise unused. ??? The bitfield needs to be able to hold all target function @@ -1692,7 +1694,10 @@ struct GTY(()) tree_function_decl { unsigned has_debug_args_flag : 1; unsigned tm_clone_flag : 1; unsigned versioned_function : 1; - /* No bits left. */ + + /* Bitfields: word 2. */ + + unsigned free_flag : 1; }; struct GTY(()) tree_translation_unit_decl { diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c index 08f10e5..d1fdb8a 100644 --- a/gcc/tree-ssa-alias.c +++ b/gcc/tree-ssa-alias.c @@ -1728,6 +1728,10 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *ref) /* Fallthru to general call handling. */; } + if (callee != NULL_TREE + && DECL_IS_FREE (callee)) + return false; + /* Check if base is a global static variable that is not read by the function. */ if (callee != NULL_TREE @@ -2117,6 +2121,13 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref) /* Fallthru to general call handling. */; } + if (callee != NULL_TREE + && DECL_IS_FREE (callee)) + { + tree ptr = gimple_call_arg (call, 0); + return ptr_deref_may_alias_ref_p_1 (ptr, ref); + } + /* Check if base is a global static variable that is not written by the function. */ if (callee != NULL_TREE @@ -2332,16 +2343,6 @@ stmt_kills_ref_p (gimple *stmt, ao_ref *ref) && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) switch (DECL_FUNCTION_CODE (callee)) { - case BUILT_IN_FREE: - { - tree ptr = gimple_call_arg (stmt, 0); - tree base = ao_ref_base (ref); - if (base && TREE_CODE (base) == MEM_REF - && TREE_OPERAND (base, 0) == ptr) - return true; - break; - } - case BUILT_IN_MEMCPY: case BUILT_IN_MEMPCPY: case BUILT_IN_MEMMOVE: @@ -2402,6 +2403,16 @@ stmt_kills_ref_p (gimple *stmt, ao_ref *ref) default:; } + + if (callee != NULL_TREE + && DECL_IS_FREE (callee)) + { + tree ptr = gimple_call_arg (stmt, 0); + tree base = ao_ref_base (ref); + if (base && TREE_CODE (base) == MEM_REF + && TREE_OPERAND (base, 0) == ptr) + return true; + } } return false; } diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c index 91c72eb..9906981 100644 --- a/gcc/tree-streamer-in.c +++ b/gcc/tree-streamer-in.c @@ -324,6 +324,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_FREE (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 c37755d..20f9ae6 100644 --- a/gcc/tree-streamer-out.c +++ b/gcc/tree-streamer-out.c @@ -292,6 +292,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_FREE (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 fa70596..b3dc5dd 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2786,6 +2786,12 @@ 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 + if it were free or C++ operator delete (first parameter is the object being + freed) for the purpose of DSE. */ +#define DECL_IS_FREE(NODE) \ + (FUNCTION_DECL_CHECK (NODE)->function_decl.free_flag) + /* Nonzero in a FUNCTION_DECL means this function may return more than once. */ #define DECL_IS_RETURNS_TWICE(NODE) \