This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix missed DSE opportunity with operator delete.
- From: Mikhail Maltsev <maltsevm at gmail dot com>
- To: gcc-patches mailing list <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>
- Date: Sun, 17 Apr 2016 00:32:52 +0300
- Subject: [PATCH] Fix missed DSE opportunity with operator delete.
- Authentication-results: sourceware.org; auth=none
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 <maltsevm@gmail.com>
* c-decl.c (merge_decls): Handle free_flag.
gcc/ChangeLog:
2016-04-16 Mikhail Maltsev <maltsevm@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 <maltsevm@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 <maltsevm@gmail.com>
* c-common.c (handle_free_attribute): New function.
gcc/cp/ChangeLog:
2016-04-16 Mikhail Maltsev <maltsevm@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) \