From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30380 invoked by alias); 29 Jul 2019 14:39:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 30372 invoked by uid 89); 29 Jul 2019 14:39:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=harmful X-HELO: mail-lf1-f68.google.com Received: from mail-lf1-f68.google.com (HELO mail-lf1-f68.google.com) (209.85.167.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 29 Jul 2019 14:39:21 +0000 Received: by mail-lf1-f68.google.com with SMTP id c19so42238542lfm.10 for ; Mon, 29 Jul 2019 07:39:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=aH6PsKwNABDxmhD8BY1XxKxLm5996FAxpWFFP8wAV6k=; b=s0koVjG2I9DQH2lgHRWzbi8daG9BJnm0NT2yrDgplZYH1m5Y8T7BBQjq3rwr0vj8hx opFBijX+Y+9rCHwS1HRBy74j2Z1GyZea1aEptnw101qT56sexRscmznAZ/RdNxSlZOZE iOi05g5d2F/jbogCAhBKn2jAF0agmYcUeoduZDHZoDx6pdZrrmxHNeu9edcZboMhRs6H vDdvtcVcSvzDXhMtqh2TaVsAWkRJppa1IMX13z4z3LidoH7sD6LwLTmXJpyKej7g44q7 TJf8EgFhqGr7alrzbdqYQ+GbOlGE/eNtnHjBJYSrkjchGfXAxNu2iJ7RBcSHpc7gb7lE Txow== MIME-Version: 1.0 References: <8305B5F4-2A96-4698-8C2E-3255658B5C12@theobroma-systems.com> <9D090495-3C97-4873-B0DF-2610B478F621@gmail.com> <70688da3-caf4-53c1-d0ee-63d16cbaadd9@suse.cz> <12f00f76-31c1-d3ca-a71b-c14b85892ef5@suse.cz> <6b43a610-4a16-cd1a-b7fa-ef2da7a77729@redhat.com> <74ce7d0b-2610-8cb6-4c22-60f9ed2bfc23@suse.cz> <3a940ca7-b024-61d1-3b05-6d36d3e22f25@suse.cz> <6c25e7d2-81eb-fb59-bd44-839c70cdebcd@suse.cz> In-Reply-To: From: Richard Biener Date: Mon, 29 Jul 2019 14:40:00 -0000 Message-ID: Subject: Re: [PATCH] Remove also 2nd argument for unused delete operator (PR tree-optimization/91270). To: =?UTF-8?Q?Martin_Li=C5=A1ka?= Cc: Marc Glisse , Jason Merrill , GCC Patches , David Malcolm , dominik.infuehr@theobroma-systems.com, Nathan Sidwell Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg01732.txt.bz2 On Mon, Jul 29, 2019 at 12:37 PM Martin Li=C5=A1ka wrote: > > On 7/29/19 11:59 AM, Richard Biener wrote: > > On Sun, Jul 28, 2019 at 4:57 PM Martin Li=C5=A1ka wrot= e: > >> > >> Hi. > >> > >> And there's one more patch that deals with delete operator > >> which has a second argument (size). That definition GIMPLE > >> statement of the size must be also properly marked. > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > This isn't a proper fix. You can't mark a random operand as necessary > > during elimination - this only works in very constraint cases. Here > > it will break down if the stmt you just marked depends on another stmt > > only used by the stmt you just marked (and thus also not necessary). > > Ah, I see. > > > > > The fix is to propagate_necessity where you either have to excempt > > the two-operator delete from handling > > We want to process also these delete operators. > > > or to mark its second operand > > as necessary despite eventually deleting the call. > > Really marking that as necessary? Why? > > > You can avoid > > this in case the allocation function used the exact same size argument. > > That's not the case as the operator new happens in a loop: > > : > # iftmp.0_15 =3D PHI > _23 =3D operator new [] (iftmp.0_15); > _24 =3D _23; > MEM[(sizetype *)_24] =3D _19; > _26 =3D _24 + 8; > _27 =3D _26; > _2 =3D _19 + 18446744073709551615; > _28 =3D (long int) _2; > > : > # _12 =3D PHI <_27(5), _29(7)> > # _13 =3D PHI <_28(5), _30(7)> > if (_13 < 0) > goto ; [INV] > else > goto ; [INV] > > Btw. is the hunk moved to propagate_necessity still wrong: > > diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c > index cf507fa0453..1c890889932 100644 > --- a/gcc/tree-ssa-dce.c > +++ b/gcc/tree-ssa-dce.c > @@ -803,7 +803,23 @@ propagate_necessity (bool aggressive) > || DECL_FUNCTION_CODE (def_callee) =3D=3D BUIL= T_IN_MALLOC > || DECL_FUNCTION_CODE (def_callee) =3D=3D BUIL= T_IN_CALLOC)) > || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee))) > - continue; > + { > + /* Some delete operators have 2 arguments, where > + the second argument is size of the deallocated memor= y. */ > + if (gimple_call_num_args (stmt) =3D=3D 2) Please make sure this only matches operator delete (just make the check we already do above also set a bool flag). > + { > + tree ptr =3D gimple_call_arg (stmt, 1); > + if (TREE_CODE (ptr) =3D=3D SSA_NAME) you can use mark_operand_necessary (ptr) here (but please name 'ptr' differently ;)) And note you can elide this in case the size arguments to new and delete match. I notice the above also matches ptr =3D malloc (4); delete ptr; not sure if intended (or harmful). Richard. > + { > + gimple *def_stmt =3D SSA_NAME_DEF_STMT (ptr); > + if (!gimple_nop_p (def_stmt) > + && !gimple_plf (def_stmt, STMT_NECESSARY)) > + gimple_set_plf (stmt, STMT_NECESSARY, false); > + } > + } > + > + continue; > + } > } > > FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE) > > Thanks, > Martin > > > > > Richard. > > > >> Thanks, > >> Martin >