From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x234.google.com (mail-lj1-x234.google.com [IPv6:2a00:1450:4864:20::234]) by sourceware.org (Postfix) with ESMTPS id CA8073858C50 for ; Tue, 21 May 2024 06:07:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CA8073858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CA8073858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::234 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716271650; cv=none; b=ppeMa/mwLbN5g070sTb3CP3J3EaMxK7ymjRVdgcsZVlhUk5hnzUlbCn3ylKsKBwGKpOazwS4vjedEP/xDnsXrvXL9KZGGjASaKnDVenoy9QCGSHB6frfZj4Bd3sSNgJWSLsjJzf5zMA6FQSGqSZyaA08Z+lbsq0EIkrkse0xiJc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716271650; c=relaxed/simple; bh=9He8e7a1glnpgxgOfT93aZ56kzIVT+aLHUvBPltJrPc=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=BEcieKfQqalEjo1oPlBSDHXaldj07lTW+sN5JEJGdBgNAbidvhZaw3kllrsPDVgz2UNfNJqbrX+P0l76Sy0fe5tlJIe0neP0cVaEbIWp2/2emT6tFT8gRDMcFB5iFyZw3yq8/Vrr0JNvkrM2Nxtvi+vAK8kqBpZCzuV0gXtd5Y4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x234.google.com with SMTP id 38308e7fff4ca-2e6f33150bcso51402201fa.2 for ; Mon, 20 May 2024 23:07:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716271646; x=1716876446; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=g0tCHoT3ucmuqeTTq3Ncbhwm2bXchtYBXEDHxyD94Cw=; b=NYZGoWS65H44ms+iWlrdFsBmPCl93ZZ+ljQMoewHyG0GHDze1hZnDMdCyJGWF7fN4s Fi1217UKR3U7aJ46C5h8peKajd/s1kkRnruBGoMhCemb686F/wz9lc4erySKllLMnYyw u1Mx01fsOPa+ZlzMtpMl6Ps/W5DlVvxQF8kMHJOw6M+kz8T6t6pkPysw6ppAhepYHmTp +afCZsBL9makDN7j59XaYY/DhvwtFPExiOBCbeNu0PuW63R78V+MQSVCkNVi/fm0A+zv hCvjrVWpeXtfE63ghKWjszAQM3P63z0oGr4GljcfCR8PDEVCq744TBthj3+kA49yG3eL LeLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716271646; x=1716876446; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=g0tCHoT3ucmuqeTTq3Ncbhwm2bXchtYBXEDHxyD94Cw=; b=YCLxaRG8B6smcb9PksMcsZVv7X8FObg0TVAJOdkjyQLQmB2Wn77Tn8jqTiJTo94TZV xhs1crrMvESneKoaeS+9rdIPPEYvRTTmsXjRjObvV/sshWUz83vs+xJH+XyZ06CV5MVK 3+D6SqrLUrFbcRn1RPcXAnIXcaERb7KNZxwrNeb3wStqDoeiQdkBTejw01MGxWY8+CwR 4dt5hDM4wjKoWhS3/1BgFWWXF0YocBAhz7QahaUvbZVJTbSm7Jn3WrjUkP/8jNDD9adI eQ5EpawLU1ciTv4jM009u1b8eK0qpGOi/ZgoJZjGN0GZ9UYdi4VmUsji00dW5I8lR95N GAVg== X-Gm-Message-State: AOJu0YzqTddNaPgkl+wnUgmUw4oOBrUcgc24x5RiP41Y76tGWHqmZfv8 j54jl+fsQhlWnxH2K1j+1iEcFjZARjBJRpeuGDKwjF9Id9iWu6WQOHcZZBH76H66vP0+qhLdkzX mlPvy8BEcxf29y2g89aiUHmDglrw= X-Google-Smtp-Source: AGHT+IEAF9BM8fZ/Dv6pYSSj2FIvxt57RfObuL1tckd5Q7e8DtsnxU+x0VZ48LzjWlQOZXIWx6xVdyddXipqNBwVkjU= X-Received: by 2002:a2e:a9aa:0:b0:2e7:14a4:1f71 with SMTP id 38308e7fff4ca-2e714a43157mr68778191fa.43.1716271645897; Mon, 20 May 2024 23:07:25 -0700 (PDT) MIME-Version: 1.0 References: <20240518231146.954808-1-quic_apinski@quicinc.com> In-Reply-To: From: Richard Biener Date: Tue, 21 May 2024 08:07:14 +0200 Message-ID: Subject: Re: [PATCH] PHIOPT: Don't transform minmax if middle bb contains a phi [PR115143] To: "Andrew Pinski (QUIC)" Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, May 20, 2024 at 11:37=E2=80=AFPM Andrew Pinski (QUIC) wrote: > > > -----Original Message----- > > From: Richard Biener > > Sent: Sunday, May 19, 2024 11:55 AM > > To: Andrew Pinski (QUIC) > > Cc: gcc-patches@gcc.gnu.org > > Subject: Re: [PATCH] PHIOPT: Don't transform minmax if > > middle bb contains a phi [PR115143] > > > > > > > > > Am 19.05.2024 um 01:12 schrieb Andrew Pinski > > : > > > > > > =EF=BB=BFThe problem here is even if last_and_only_stmt returns a > > statement, > > > the bb might still contain a phi node which defines a ssa > > name which > > > is used in that statement so we need to add a check to make > > sure that > > > the phi nodes are empty for the middle bbs in both the > > > `CMP?MINMAX:MINMAX` case and the `CMP?MINMAX:B` > > cases. > > > > Is that single arg PHIs or do we have an extra edge into the > > middle BB? I think that might be unexpected, at least costing > > wise. Maybe Also to some of the replacement code we have ? > > It is only a single arg PHI since we already reject multiple edges in the= middle BBs for these cases. > It was EVPR that produces the single arg PHI in the original testcase fro= m folding of a conditional to false and evpr does not do simple name prop i= n this case and there is no pass inbetween evrp and phiopt that will clear = up single arg PHI. > I added the Gimple based testcases basically to avoid the needing of depe= nding on what previous passes could produce too. > > > > > > OK for trunk and backport to all open branches since r14- > > 3827-g30e6ee074588ba was backported? > > > Bootstrapped and tested on x86_64_linux-gnu with no > > regressions. > > > > > > > Ok > > Does this include the GCC 13 branch or should I wait until after the GCC = 13.3.0 release? Please wait until after the release. Thanks, Richard. > Thanks, > Andrew Pinski > > > > > Richard > > > > > PR tree-optimization/115143 > > > > > > gcc/ChangeLog: > > > > > > * tree-ssa-phiopt.cc (minmax_replacement): Check for > > empty > > > phi nodes for middle bbs for the case where middle bb is > > not empty. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.c-torture/compile/pr115143-1.c: New test. > > > * gcc.c-torture/compile/pr115143-2.c: New test. > > > * gcc.c-torture/compile/pr115143-3.c: New test. > > > > > > Signed-off-by: Andrew Pinski > > > --- > > > .../gcc.c-torture/compile/pr115143-1.c | 21 > > +++++++++++++ > > > .../gcc.c-torture/compile/pr115143-2.c | 30 > > +++++++++++++++++++ > > > .../gcc.c-torture/compile/pr115143-3.c | 29 > > ++++++++++++++++++ > > > gcc/tree-ssa-phiopt.cc | 12 ++++++++ > > > 4 files changed, 92 insertions(+) > > > create mode 100644 gcc/testsuite/gcc.c- > > torture/compile/pr115143-1.c > > > create mode 100644 gcc/testsuite/gcc.c- > > torture/compile/pr115143-2.c > > > create mode 100644 gcc/testsuite/gcc.c- > > torture/compile/pr115143-3.c > > > > > > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c > > > b/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c > > > new file mode 100644 > > > index 00000000000..5cb119ea432 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c > > > @@ -0,0 +1,21 @@ > > > +/* PR tree-optimization/115143 */ > > > +/* This used to ICE. > > > + minmax part of phiopt would transform, > > > + would transform `a!=3D0?min(a, b) : 0` into `min(a,b)` > > > + which was correct except b was defined by a phi in the > > inner > > > + bb which was not handled. */ > > > +short a, d; > > > +char b; > > > +long c; > > > +unsigned long e, f; > > > +void g(unsigned long h) { > > > + if (c ? e : b) > > > + if (e) > > > + if (d) { > > > + a =3D f ? ({ > > > + unsigned long i =3D d ? f : 0, j =3D e ? h : 0; > > > + i < j ? i : j; > > > + }) : 0; > > > + } > > > +} > > > + > > > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c > > > b/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c > > > new file mode 100644 > > > index 00000000000..05c3bbe9738 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c > > > @@ -0,0 +1,30 @@ > > > +/* { dg-options "-fgimple" } */ > > > +/* PR tree-optimization/115143 */ > > > +/* This used to ICE. > > > + minmax part of phiopt would transform, > > > + would transform `a!=3D0?min(a, b) : 0` into `min(a,b)` > > > + which was correct except b was defined by a phi in the > > inner > > > + bb which was not handled. */ > > > +unsigned __GIMPLE (ssa,startwith("phiopt")) foo (unsigned > > a, unsigned > > > +b) { > > > + unsigned j; > > > + unsigned _23; > > > + unsigned _12; > > > + > > > + __BB(2): > > > + if (a_6(D) !=3D 0u) > > > + goto __BB3; > > > + else > > > + goto __BB4; > > > + > > > + __BB(3): > > > + j_10 =3D __PHI (__BB2: b_11(D)); > > > + _23 =3D __MIN (a_6(D), j_10); > > > + goto __BB4; > > > + > > > + __BB(4): > > > + _12 =3D __PHI (__BB3: _23, __BB2: 0u); return _12; > > > + > > > +} > > > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c > > > b/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c > > > new file mode 100644 > > > index 00000000000..53c5fb5588e > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c > > > @@ -0,0 +1,29 @@ > > > +/* { dg-options "-fgimple" } */ > > > +/* PR tree-optimization/115143 */ > > > +/* This used to ICE. > > > + minmax part of phiopt would transform, > > > + would transform `a!=3D0?min(a, b) : 0` into `min(a,b)` > > > + which was correct except b was defined by a phi in the > > inner > > > + bb which was not handled. */ > > > +unsigned __GIMPLE (ssa,startwith("phiopt")) foo (unsigned > > a, unsigned > > > +b) { > > > + unsigned j; > > > + unsigned _23; > > > + unsigned _12; > > > + > > > + __BB(2): > > > + if (a_6(D) > 0u) > > > + goto __BB3; > > > + else > > > + goto __BB4; > > > + > > > + __BB(3): > > > + j_10 =3D __PHI (__BB2: b_7(D)); > > > + _23 =3D __MIN (a_6(D), j_10); > > > + goto __BB4; > > > + > > > + __BB(4): > > > + _12 =3D __PHI (__BB3: _23, __BB2: 0u); > > > + return _12; > > > +} > > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > > index > > > f166c3132cb..918cf50b589 100644 > > > --- a/gcc/tree-ssa-phiopt.cc > > > +++ b/gcc/tree-ssa-phiopt.cc > > > @@ -1925,6 +1925,10 @@ minmax_replacement > > (basic_block cond_bb, basic_block middle_bb, basic_block > > alt_ > > > || gimple_code (assign) !=3D GIMPLE_ASSIGN) > > > return false; > > > > > > + /* There cannot be any phi nodes in the middle bb. */ > > > + if (!gimple_seq_empty_p (phi_nodes (middle_bb))) > > > + return false; > > > + > > > lhs =3D gimple_assign_lhs (assign); > > > ass_code =3D gimple_assign_rhs_code (assign); > > > if (ass_code !=3D MAX_EXPR && ass_code !=3D MIN_EXPR) > > @@ -1938,6 > > > +1942,10 @@ minmax_replacement (basic_block cond_bb, > > basic_block middle_bb, basic_block alt_ > > > || gimple_code (assign) !=3D GIMPLE_ASSIGN) > > > return false; > > > > > > + /* There cannot be any phi nodes in the alt middle bb. > > */ > > > + if (!gimple_seq_empty_p (phi_nodes (alt_middle_bb))) > > > + return false; > > > + > > > alt_lhs =3D gimple_assign_lhs (assign); > > > if (ass_code !=3D gimple_assign_rhs_code (assign)) > > > return false; > > > @@ -2049,6 +2057,10 @@ minmax_replacement > > (basic_block cond_bb, basic_block middle_bb, basic_block > > alt_ > > > || gimple_code (assign) !=3D GIMPLE_ASSIGN) > > > return false; > > > > > > + /* There cannot be any phi nodes in the middle bb. */ > > > + if (!gimple_seq_empty_p (phi_nodes (middle_bb))) > > > + return false; > > > + > > > lhs =3D gimple_assign_lhs (assign); > > > ass_code =3D gimple_assign_rhs_code (assign); > > > if (ass_code !=3D MAX_EXPR && ass_code !=3D MIN_EXPR) > > > -- > > > 2.43.0 > > >