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 C1147385BF83 for ; Tue, 14 Apr 2020 07:09:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C1147385BF83 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 BA817AB5C; Tue, 14 Apr 2020 07:09:43 +0000 (UTC) Subject: Re: [PATCH] Allow new/delete operator deletion only for replaceable. To: Jakub Jelinek Cc: Jonathan Wakely , Marc Glisse , Richard Biener , Jason Merrill , Jan Hubicka , GCC Patches , Nathan Sidwell References: <20200408133252.GG2212@tucnak> <20d175a6-23df-43e5-7027-d11fc660abd1@suse.cz> <8cac5c65-3b93-1c9a-87e9-9e42eb876eba@suse.cz> <12cf42c1-88f9-7a08-fad8-b9830bd20fcf@suse.cz> <20200410091754.GQ2212@tucnak> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <069c1c26-d74a-0eea-554b-1aeb6881dca7@suse.cz> Date: Tue, 14 Apr 2020 09:09:43 +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: <20200410091754.GQ2212@tucnak> Content-Type: multipart/mixed; boundary="------------A911ED3D72C4E643A6C54C46" Content-Language: en-US X-Spam-Status: No, score=-27.9 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: Tue, 14 Apr 2020 07:09:47 -0000 This is a multi-part message in MIME format. --------------A911ED3D72C4E643A6C54C46 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit On 4/10/20 11:17 AM, Jakub Jelinek wrote: > On Fri, Apr 10, 2020 at 10:29:29AM +0200, Martin Liška wrote: >> +/* 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" , > > Formatting, I think the /* and "_Z should be two columns from const char, > and no space before , > Not sure I like using the : and using a hash table looks like an overkill to > me, don't we have only 8 options, and most of them with different identifier > lengths? The concat itself will take a while... > And, what's worse, the m in there is really different on different targets. > It can be j, m or y depending on what fundamental type size_t is. > > So why not (completely untested): Hi. I've updated a bit the Jakub's patch. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin > > static bool > valid_new_delete_pair_p (gimple *new_call, gimple *delete_call) > { > tree new_asm = DECL_ASSEMBLER_NAME (gimple_call_fndecl (new_call)); > tree delete_asm = DECL_ASSEMBLER_NAME (gimple_call_fndecl (delete_call)); > const char *new_name = IDENTIFIER_POINTER (new_asm); > const char *delete_name = IDENTIFIER_POINTER (delete_asm); > unsigned int new_len = IDENTIFIER_LENGTH (mew_asm); > unsigned int delete_len = IDENTIFIER_LENGTH (delete_asm); > if (new_name < 5 || delete_len < 6) > return false; > if (new_name[0] == '_') > ++new_name, --new_len; > if (new_name[0] == '_') > ++new_name, --new_len; > if (delete_name[0] == '_') > ++delete_name, --delete_len; > if (delete_name[0] == '_') > ++delete_name, --delete_len; > if (new_len < 4 || delete_len < 5) > return false; > /* *_len is now just the length after initial underscores. */ > if (new_name[0] != 'Z' || new_name[1] != 'n') > return false; > if (delete_name[0] != 'Z' || delete_name[1] != 'd') > return false; > /* _Znw must match _Zdl, _Zna must match _Zda. */ > if ((new_name[2] != 'w' || delete_name[2] != 'l') > && (new_name[2] != 'a' || delete_name[2] != 'a')) > return false; > if (new_name[3] != 'j' && new_name[3] != 'm' && new_name[3] != 'y') > return false; > if (delete_name[3] != 'P' || delete_name[4] != 'v') > return false; > if (new_len == 4 > || (new_len == 18 && !memcmp (new_name + 4, "RKSt9nothrow_t", 14))) > { > /* _ZnXY or _ZnXYRKSt9nothrow_t matches > _ZdXPv, _ZdXPvY and _ZdXPvRKSt9nothrow_t. */ > if (delete_len == 5) > return true; > if (delete_len == 6 && delete_name[5] == new_name[3]) > return true; > if (delete_len == 19 && !memcmp (delete_name + 5, "RKSt9nothrow_t", 14)) > return true; > } > else if ((new_len == 19 && !memcmp (new_name + 4, "St11align_val_t", 15)) > || (new_len == 33 > && !memcmp (new_name + 4, "St11align_val_tRKSt9nothrow_t", 29))) > { > /* _ZnXYSt11align_val_t or _ZnXYSt11align_val_tRKSt9nothrow_t matches > _ZdXPvSt11align_val_t or _ZdXPvYSt11align_val_t or or > _ZdXPvSt11align_val_tRKSt9nothrow_t. */ > if (delete_len == 20 && !memcmp (delete_name + 5, "St11align_val_t", 15)) > return true; > if (delete_len == 21 > && delete_name[5] == new_name[3] > && !memcmp (delete_name + 6, "St11align_val_t", 15)) > return true; > if (delete_len == 34 > && !memcmp (delete_name + 5, "St11align_val_tRKSt9nothrow_t", 29)) > return true; > } > return false; > } > > Jakub > --------------A911ED3D72C4E643A6C54C46 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 60626d4b040bc502c563b7c014b62751a517705a 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 Jakub Jelinek 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 Jakub Jelinek 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 | 103 ++++++++++++++++++++++++++++--- 3 files changed, 128 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..69566cd50e2 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -646,6 +646,80 @@ 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) +{ + tree new_asm = DECL_ASSEMBLER_NAME (gimple_call_fndecl (new_call)); + tree delete_asm = DECL_ASSEMBLER_NAME (gimple_call_fndecl (delete_call)); + const char *new_name = IDENTIFIER_POINTER (new_asm); + const char *delete_name = IDENTIFIER_POINTER (delete_asm); + unsigned int new_len = IDENTIFIER_LENGTH (new_asm); + unsigned int delete_len = IDENTIFIER_LENGTH (delete_asm); + + if (new_len < 5 || delete_len < 6) + return false; + if (new_name[0] == '_') + ++new_name, --new_len; + if (new_name[0] == '_') + ++new_name, --new_len; + if (delete_name[0] == '_') + ++delete_name, --delete_len; + if (delete_name[0] == '_') + ++delete_name, --delete_len; + if (new_len < 4 || delete_len < 5) + return false; + /* *_len is now just the length after initial underscores. */ + if (new_name[0] != 'Z' || new_name[1] != 'n') + return false; + if (delete_name[0] != 'Z' || delete_name[1] != 'd') + return false; + /* _Znw must match _Zdl, _Zna must match _Zda. */ + if ((new_name[2] != 'w' || delete_name[2] != 'l') + && (new_name[2] != 'a' || delete_name[2] != 'a')) + return false; + /* 'j', 'm' and 'y' correspond to size_t. */ + if (new_name[3] != 'j' && new_name[3] != 'm' && new_name[3] != 'y') + return false; + if (delete_name[3] != 'P' || delete_name[4] != 'v') + return false; + if (new_len == 4 + || (new_len == 18 && !memcmp (new_name + 4, "RKSt9nothrow_t", 14))) + { + /* _ZnXY or _ZnXYRKSt9nothrow_t matches + _ZdXPv, _ZdXPvY and _ZdXPvRKSt9nothrow_t. */ + if (delete_len == 5) + return true; + if (delete_len == 6 && delete_name[5] == new_name[3]) + return true; + if (delete_len == 19 && !memcmp (delete_name + 5, "RKSt9nothrow_t", 14)) + return true; + } + else if ((new_len == 19 && !memcmp (new_name + 4, "St11align_val_t", 15)) + || (new_len == 33 + && !memcmp (new_name + 4, "St11align_val_tRKSt9nothrow_t", 29))) + { + /* _ZnXYSt11align_val_t or _ZnXYSt11align_val_tRKSt9nothrow_t matches + _ZdXPvSt11align_val_t or _ZdXPvYSt11align_val_t or or + _ZdXPvSt11align_val_tRKSt9nothrow_t. */ + if (delete_len == 20 && !memcmp (delete_name + 5, "St11align_val_t", 15)) + return true; + if (delete_len == 21 + && delete_name[5] == new_name[3] + && !memcmp (delete_name + 6, "St11align_val_t", 15)) + return true; + if (delete_len == 34 + && !memcmp (delete_name + 5, "St11align_val_tRKSt9nothrow_t", 29)) + return true; + } + return false; +} + /* 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 +898,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 +1743,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 --------------A911ED3D72C4E643A6C54C46--