From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 3CE5C385B835 for ; Fri, 10 Apr 2020 08:29:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3CE5C385B835 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 38DF5AE1A; Fri, 10 Apr 2020 08:29:30 +0000 (UTC) Subject: Re: [PATCH] Allow new/delete operator deletion only for replaceable. To: Jonathan Wakely Cc: Marc Glisse , Richard Biener , Jason Merrill , Jakub Jelinek , Jan Hubicka , GCC Patches , Nathan Sidwell References: <20200403152609.GA35629@kam.mff.cuni.cz> <20200408133252.GG2212@tucnak> <20d175a6-23df-43e5-7027-d11fc660abd1@suse.cz> <8cac5c65-3b93-1c9a-87e9-9e42eb876eba@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <12cf42c1-88f9-7a08-fad8-b9830bd20fcf@suse.cz> Date: Fri, 10 Apr 2020 10:29:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------4F677C9AA44F229572E0D1D2" Content-Language: en-US X-Spam-Status: No, score=-28.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Apr 2020 08:29:33 -0000 This is a multi-part message in MIME format. --------------4F677C9AA44F229572E0D1D2 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 4/10/20 10:18 AM, Jonathan Wakely wrote: > On Fri, 10 Apr 2020 at 09:08, Martin Liška wrote: >> Marc pointed out that some targets do not use the leading '_' (or use 2 dashes?) for mangled named? > > Double underscore at the start. I think darwin uses that. > Ah yeah, not dashes, but underscores ;) It's fixed in attached patch that I've been testing right now. Martin --------------4F677C9AA44F229572E0D1D2 Content-Type: text/x-patch; charset=UTF-8; name="0001-List-valid-pairs-for-new-and-delete-operators.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-List-valid-pairs-for-new-and-delete-operators.patch" >From 4043b187ee8e6d7d0c05a6463e38ca54ba4acf75 Mon Sep 17 00:00:00 2001 From: Martin Liska 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 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 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 | 99 ++++++++++++++++++++++++++++---- 3 files changed, 124 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 + +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..f66069e410d 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -646,6 +646,76 @@ degenerate_phi_p (gimple *phi) return true; } +/* Valid pairs of new and delete operators for DCE. */ +static hash_set *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))); + + if (new_name[0] == '_' and new_name[1] == '_') + ++new_name; + if (delete_name[0] == '_' and delete_name[1] == '_') + ++delete_name; + + char *needle = concat (new_name, ":", delete_name, NULL); + + if (valid_pairs == NULL) + { + valid_pairs = new hash_set (); + /* 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 +894,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 +1739,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 --------------4F677C9AA44F229572E0D1D2--