On 4/9/20 10:13 AM, Jonathan Wakely wrote:
On Thu, 9 Apr 2020 at 09:05, Marc Glisse wrote:
Note that the matching is not 1-to-1. Array vs non-array and
aligned vs non-aligned seem important, but sized and unsized delete can
both match the same new, IIUC.
Right.
Not sure about the nothrow versions...
This is valid, and mixes the nothrow new with non-nothrow delete:
delete new (std::nothrow) int;
All right, there's a patch candidate that comes up with the list of possible
pairs.
For better readability, I present demangled names:
$ cat /tmp/pairs.txt | c++filt
"operator new(unsigned long):operator delete(void*)" ,
"operator new(unsigned long):operator delete(void*, unsigned long)" ,
"operator new(unsigned long):operator delete(void*, std::nothrow_t
const&)" ,
"operator new(unsigned long, std::nothrow_t const&):operator
delete(void*)" ,
"operator new(unsigned long, std::nothrow_t const&):operator delete(void*,
unsigned long)" ,
"operator new(unsigned long, std::nothrow_t const&):operator delete(void*,
std::nothrow_t const&)" ,
/* non-[] operators with alignment. */
"operator new(unsigned long, std::align_val_t):operator delete(void*,
unsigned long, std::align_val_t)" ,
"operator new(unsigned long, std::align_val_t):operator delete(void*,
std::align_val_t)" ,
"operator new(unsigned long, std::align_val_t):operator delete(void*,
std::align_val_t, std::nothrow_t const&)" ,
"operator new(unsigned long, std::align_val_t, std::nothrow_t
const&):operator delete(void*, unsigned long, std::align_val_t)" ,
"operator new(unsigned long, std::align_val_t, std::nothrow_t
const&):operator delete(void*, std::align_val_t)" ,
"operator new(unsigned long, std::align_val_t, std::nothrow_t const&):operator
delete(void*, std::align_val_t, std::nothrow_t const&)" ,
/* [] operators. */
"operator new[](unsigned long):operator delete[](void*)" ,
"operator new[](unsigned long):operator delete[](void*, unsigned
long)" ,
"operator new[](unsigned long):operator delete[](void*, std::nothrow_t
const&)" ,
"operator new[](unsigned long, std::nothrow_t const&):operator
delete[](void*)" ,
"operator new[](unsigned long, std::nothrow_t const&):operator
delete[](void*, unsigned long)" ,
"operator new[](unsigned long, std::nothrow_t const&):operator
delete[](void*, std::nothrow_t const&)" ,
/* [] operators with alignment. */
"operator new[](unsigned long, std::align_val_t):operator delete[](void*,
unsigned long, std::align_val_t)" ,
"operator new[](unsigned long, std::align_val_t):operator delete[](void*,
std::align_val_t)" ,
"operator new[](unsigned long, std::align_val_t):operator delete[](void*,
std::align_val_t, std::nothrow_t const&)" ,
"operator new[](unsigned long, std::align_val_t, std::nothrow_t
const&):operator delete[](void*, unsigned long, std::align_val_t)",
"operator new[](unsigned long, std::align_val_t, std::nothrow_t
const&):operator delete[](void*, std::align_val_t)",
"operator new[](unsigned long, std::align_val_t, std::nothrow_t
const&):operator delete[](void*, std::align_val_t, std::nothrow_t const&)",
Marc pointed out that some targets do not use the leading '_' (or use 2
dashes?) for mangled named?
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Thanks,
Martin
>From aa7102bcd698b6f132e0b9ffecd847d68083039b Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Thu, 9 Apr 2020 15:50:58 +0200
Subject: [PATCH] List valid pairs for new and delete operators.
gcc/ChangeLog:
2020-04-09 Martin Liska <mli...@suse.cz>
PR c++/94314
* cgraphclones.c (set_new_clone_decl_and_node_flags): Drop
DECL_IS_REPLACEABLE_OPERATOR during cloning.
* tree-ssa-dce.c (valid_new_delete_pair_p): New function.
(propagate_necessity): Check operator names.
(perform_tree_ssa_dce): Delete valid_pairs.
gcc/testsuite/ChangeLog:
2020-04-09 Martin Liska <mli...@suse.cz>
PR c++/94314
* g++.dg/pr94314-4.C: New test.
---
gcc/cgraphclones.c | 2 +
gcc/testsuite/g++.dg/pr94314-4.C | 33 ++++++++++++
gcc/tree-ssa-dce.c | 93 ++++++++++++++++++++++++++++----
3 files changed, 118 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr94314-4.C
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c73b8f810f0..8f541a28b6e 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -165,6 +165,7 @@ set_new_clone_decl_and_node_flags (cgraph_node *new_node)
DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
DECL_SET_IS_OPERATOR_NEW (new_node->decl, 0);
DECL_SET_IS_OPERATOR_DELETE (new_node->decl, 0);
+ DECL_IS_REPLACEABLE_OPERATOR (new_node->decl) = 0;
new_node->externally_visible = 0;
new_node->local = 1;
@@ -1030,6 +1031,7 @@ cgraph_node::create_version_clone_with_body
DECL_STATIC_DESTRUCTOR (new_decl) = 0;
DECL_SET_IS_OPERATOR_NEW (new_decl, 0);
DECL_SET_IS_OPERATOR_DELETE (new_decl, 0);
+ DECL_IS_REPLACEABLE_OPERATOR (new_decl) = 0;
/* Create the new version's call-graph node.
and update the edges of the new node. */
diff --git a/gcc/testsuite/g++.dg/pr94314-4.C b/gcc/testsuite/g++.dg/pr94314-4.C
new file mode 100644
index 00000000000..afa2a443dc4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr94314-4.C
@@ -0,0 +1,33 @@
+/* PR c++/94314. */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-cddce-details -std=c++14" } */
+/* { dg-additional-options "-fdelete-null-pointer-checks" } */
+
+#include <stdio.h>
+
+int count = 0;
+
+__attribute__((malloc, noinline)) void* operator new[](__SIZE_TYPE__ sz) {
+ ++count;
+ return ::operator new(sz);
+}
+
+void operator delete[](void* ptr) noexcept {
+ --count;
+ ::operator delete(ptr);
+}
+
+void operator delete[](void* ptr, __SIZE_TYPE__ sz) noexcept {
+ --count;
+ ::operator delete(ptr, sz);
+}
+
+int main() {
+ delete[] new int[1];
+ if (count != 0)
+ __builtin_abort ();
+
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "Deleting : operator delete" "cddce1"} } */
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index fd5f24c746c..e1074a803f8 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -646,6 +646,70 @@ degenerate_phi_p (gimple *phi)
return true;
}
+/* Valid pairs of new and delete operators for DCE. */
+static hash_set<nofree_string_hash> *valid_pairs = NULL;
+
+/* Return that NEW_CALL and DELETE_CALL are a valid pair of new
+ and delete operators. */
+
+static bool
+valid_new_delete_pair_p (gimple *new_call, gimple *delete_call)
+{
+ const char *new_name
+ = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (gimple_call_fndecl (new_call)));
+ const char *delete_name
+ = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (gimple_call_fndecl (delete_call)));
+ char *needle = concat (new_name, ":", delete_name, NULL);
+
+ if (valid_pairs == NULL)
+ {
+ valid_pairs = new hash_set<nofree_string_hash> ();
+ /* Invalid pairs:
+ non-[] and []
+ aligned and non-aligned
+ */
+
+ const char *pairs[] = {
+ /* non-[] operators. */
+ "_Znwm:_ZdlPv" ,
+ "_Znwm:_ZdlPvm" ,
+ "_Znwm:_ZdlPvRKSt9nothrow_t" ,
+ "_ZnwmRKSt9nothrow_t:_ZdlPv" ,
+ "_ZnwmRKSt9nothrow_t:_ZdlPvm" ,
+ "_ZnwmRKSt9nothrow_t:_ZdlPvRKSt9nothrow_t" ,
+ /* non-[] operators with alignment. */
+ "_ZnwmSt11align_val_t:_ZdlPvmSt11align_val_t" ,
+ "_ZnwmSt11align_val_t:_ZdlPvSt11align_val_t" ,
+ "_ZnwmSt11align_val_t:_ZdlPvSt11align_val_tRKSt9nothrow_t" ,
+ "_ZnwmSt11align_val_tRKSt9nothrow_t:_ZdlPvmSt11align_val_t" ,
+ "_ZnwmSt11align_val_tRKSt9nothrow_t:_ZdlPvSt11align_val_t" ,
+ "_ZnwmSt11align_val_tRKSt9nothrow_t:_ZdlPvSt11align_val_tRKSt9nothrow_t" ,
+ /* [] operators. */
+ "_Znam:_ZdaPv" ,
+ "_Znam:_ZdaPvm" ,
+ "_Znam:_ZdaPvRKSt9nothrow_t" ,
+ "_ZnamRKSt9nothrow_t:_ZdaPv" ,
+ "_ZnamRKSt9nothrow_t:_ZdaPvm" ,
+ "_ZnamRKSt9nothrow_t:_ZdaPvRKSt9nothrow_t" ,
+ /* [] operators with alignment. */
+ "_ZnamSt11align_val_t:_ZdaPvmSt11align_val_t" ,
+ "_ZnamSt11align_val_t:_ZdaPvSt11align_val_t" ,
+ "_ZnamSt11align_val_t:_ZdaPvSt11align_val_tRKSt9nothrow_t" ,
+ "_ZnamSt11align_val_tRKSt9nothrow_t:_ZdaPvmSt11align_val_t",
+ "_ZnamSt11align_val_tRKSt9nothrow_t:_ZdaPvSt11align_val_t",
+ "_ZnamSt11align_val_tRKSt9nothrow_t:_ZdaPvSt11align_val_tRKSt9nothrow_t",
+ NULL
+ };
+
+ for (unsigned i = 0; pairs[i] != NULL; i++)
+ valid_pairs->add (pairs[i]);
+ }
+
+ bool r = valid_pairs->contains (needle);
+ free (needle);
+ return r;
+}
+
/* Propagate necessity using the operands of necessary statements.
Process the uses on each statement in the worklist, and add all
feeding statements which contribute to the calculation of this
@@ -824,16 +888,23 @@ propagate_necessity (bool aggressive)
|| DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
|| DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
{
- /* Delete operators can have alignment and (or) size as next
- arguments. When being a SSA_NAME, they must be marked
- as necessary. */
- if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
- 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);
- }
+ if (is_delete_operator)
+ {
+ if (!valid_new_delete_pair_p (def_stmt, stmt))
+ mark_operand_necessary (gimple_call_arg (stmt, 0));
+
+ /* Delete operators can have alignment and (or) size
+ as next arguments. When being a SSA_NAME, they
+ must be marked as necessary. */
+ if (gimple_call_num_args (stmt) >= 2)
+ 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;
}
@@ -1662,6 +1733,8 @@ perform_tree_ssa_dce (bool aggressive)
visited = BITMAP_ALLOC (NULL);
propagate_necessity (aggressive);
BITMAP_FREE (visited);
+ delete valid_pairs;
+ valid_pairs = NULL;
something_changed |= eliminate_unnecessary_stmts ();
something_changed |= cfg_altered;
--
2.26.0
