Message ID | 20220713192420.3126654-1-sfeifer@redhat.com |
---|---|
State | New, archived |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a98:d5ce:0:b0:178:cc93:bf7d with SMTP id g14csp415440eik; Wed, 13 Jul 2022 12:26:14 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tOrIM6gLDz2BkRRdSoMjQGRQtgHUCrVWz3BPbI2SlAtPH30UM5Nex3pNAn87/PwXYzZTrw X-Received: by 2002:a17:907:2854:b0:72b:8f44:26a3 with SMTP id el20-20020a170907285400b0072b8f4426a3mr5014233ejc.96.1657740374390; Wed, 13 Jul 2022 12:26:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1657740374; cv=none; d=google.com; s=arc-20160816; b=yBMyVCm5t1xhlAEFozSlT4FeBdfu/WYcntFiDZxRfuUHUbWkSFYTtI92AOH/0Pehel NH+IwfPlSk9315N+3CvIgm5cd3rXsgkqIIR+HK2NoTExt2owhzJHp9rAWQV2TJvLBUIS YuLafFHJ/ulYso1kJBoUY8Pvz47Nrjl0IRDkui03rLdY1AcNcM0068TS5EFBnUDyQe4d jApWnELlZp+FZyVSdTii5r31x3tuhrjh+uVRgzlxCpEc0gpDQ04a780CV1qjNrWzmHpv BMGKZbF8lMVwrjvTSTZD8nLFZmTtjF9yegP1M9Eg1rYPiId/HSpCyCbNK/3gSeBWsH+C mqbw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=gS/0v+riODplrQoMCXlvWk0yedTtzHPl2Ea1VjSuQzc=; b=AnBR8s4PjQs8GOs8+6yU+2TJK0HJRpt50whpVm697jIcETr0LLcg0o+IDogQ2Zu+6y Ebv5VoTIQrRSkYKvyRWnU8tnXBWagDdI4m5jwdgGy0aRq/N9JEWS/Kpbp2cBgNSqdIyd mMKkWiWzxbF3G7z9nUPQo0YscecPZRn24CIgzZakkclf2epmDQ/h2ebHvZxCxawLLIhY nKk6WC6n4EphO9L5XBVZcTTBUhj6n/ujLhP7gaK+ASDYU1ZJ5hd7gSpsgNohVYsOTb/K XEg1NFNM5SWjMxkWHuFr5uzQB50Tau/wN6QNrd8CtgKRYLSkcogyaznsK61ptikJCYu1 aO+Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=jn3pNdFR; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id g10-20020a056402090a00b0043a6d29e242si3233348edz.198.2022.07.13.12.26.14 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Jul 2022 12:26:14 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=jn3pNdFR; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 68E893885512 for <ouuuleilei@gmail.com>; Wed, 13 Jul 2022 19:26:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 68E893885512 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1657740373; bh=gS/0v+riODplrQoMCXlvWk0yedTtzHPl2Ea1VjSuQzc=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=jn3pNdFRn6ljll2uV0C2zbY/Yj93bDnbC1wgq9KIZ2sFuDy8y5ePqrKkOuf4Njrv2 hKnnK/v1Gs7Qk3hr+BYPoBvk19t64Imuan52uCpktymWNuAHz76d63/VITBjLDqmOc EKZhZc+fX+aliHI6oash1H34IXHkcPEZcEjqYLNs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 707EB382C156 for <gcc-patches@gcc.gnu.org>; Wed, 13 Jul 2022 19:25:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 707EB382C156 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-663--VWSGWzbPnyMJ9iOArijuQ-1; Wed, 13 Jul 2022 15:25:15 -0400 X-MC-Unique: -VWSGWzbPnyMJ9iOArijuQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id ECCDB858F11 for <gcc-patches@gcc.gnu.org>; Wed, 13 Jul 2022 19:25:14 +0000 (UTC) Received: from sfeifer.remote.csb (unknown [10.22.18.121]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF47E18ECB for <gcc-patches@gcc.gnu.org>; Wed, 13 Jul 2022 19:25:14 +0000 (UTC) To: gcc-patches@gcc.gnu.org Subject: [PATCH] match.pd: Add new abs pattern [PR94290] Date: Wed, 13 Jul 2022 15:24:20 -0400 Message-Id: <20220713192420.3126654-1-sfeifer@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Sam Feifer via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Sam Feifer <sfeifer@redhat.com> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1738266770576167359?= X-GMAIL-MSGID: =?utf-8?q?1738266770576167359?= |
Series |
match.pd: Add new abs pattern [PR94290]
|
|
Commit Message
Sam Feifer
July 13, 2022, 7:24 p.m. UTC
This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. Tests are also included to be added to the testsuite. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR tree-optimization/94290 gcc/ChangeLog: * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. * match.pd (x <= 0 ? -x : 0): New Simplification. gcc/testsuite/ChangeLog: * gcc.c-torture/execute/pr94290-1.c: New test. * gcc.dg/pr94290-2.c: New test. * gcc.dg/pr94290.c: New test. --- gcc/match.pd | 15 ++++++ .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c create mode 100644 gcc/testsuite/gcc.dg/pr94290.c base-commit: 6af530f914801f5e561057da55c41480f28751f7
Comments
On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. You could use :c for the commutative property instead and that should simplify things. That is: (simplify (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) (abs @0)) Also since integer_zerop works on vectors, it seems like you should add a testcase or two for the vector case. Also would be useful if you write a testcase that uses different statements rather than one big one so it gets exercised in the forwprop case. Note also if either of the max are used more than just in this simplification, it could increase the lifetime of @0, maybe you need to add :s to the max expressions. Thanks, Andrew > > Tests are also included to be added to the testsuite. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR tree-optimization/94290 > > gcc/ChangeLog: > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. > * match.pd (x <= 0 ? -x : 0): New Simplification. > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/execute/pr94290-1.c: New test. > * gcc.dg/pr94290-2.c: New test. > * gcc.dg/pr94290.c: New test. > --- > gcc/match.pd | 15 ++++++ > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > 4 files changed, 92 insertions(+) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 45aefd96688..55ca79d7ac9 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7848,3 +7848,18 @@ and, > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > (bit_and @0 @1) > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > + > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > +(simplify > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > + (abs @0)) > + > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > +(simplify > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > + (abs @0)) > + > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > + (max (negate @0) @1)) > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > new file mode 100644 > index 00000000000..93b80d569aa > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > @@ -0,0 +1,16 @@ > +/* PR tree-optimization/94290 */ > + > +#include "../../gcc.dg/pr94290.c" > + > +int main() { > + > + if (foo(0) != 0 > + || foo(-42) != 42 > + || foo(42) != 42 > + || baz(-10) != 10 > + || baz(-10) != 10) { > + __builtin_abort(); > + } > + > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c > new file mode 100644 > index 00000000000..ea6e55755f5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/94290 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > + > +/* Form from PR. */ > +__attribute__((noipa)) unsigned int foo(int x) { > + return x <= 0 ? -x : 0; > +} > + > +/* Changed order. */ > +__attribute__((noipa)) unsigned int bar(int x) { > + return 0 >= x ? -x : 0; > +} > + > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c > new file mode 100644 > index 00000000000..47617c36c02 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr94290.c > @@ -0,0 +1,46 @@ > +/* PR tree-optimization/94290 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > + > + > +/* Same form as PR. */ > +__attribute__((noipa)) unsigned int foo(int x) { > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > +} > + > +/* Signed function. */ > +__attribute__((noipa)) int bar(int x) { > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > +} > + > +/* Commutative property. */ > +__attribute__((noipa)) unsigned int baz(int x) { > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > +} > + > +/* Flipped order for max expressions. */ > +__attribute__((noipa)) unsigned int quux(int x) { > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > +} > + > +/* Not zero so should not optimize. */ > +__attribute__((noipa)) unsigned int waldo(int x) { > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > +} > + > +/* Not zero so should not optimize. */ > +__attribute__((noipa)) unsigned int fred(int x) { > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > +} > + > +/* Incorrect pattern. */ > +__attribute__((noipa)) unsigned int goo(int x) { > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > +} > + > +/* Incorrect pattern. */ > +__attribute__((noipa)) int qux(int x) { > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > +} > + > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > -- > 2.31.1 >
On Wed, Jul 13, 2022 at 9:36 PM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. > > You could use :c for the commutative property instead and that should > simplify things. > That is: > > (simplify > (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > (abs @0)) > > Also since integer_zerop works on vectors, it seems like you should > add a testcase or two for the vector case. > Also would be useful if you write a testcase that uses different > statements rather than one big one so it gets exercised in the > forwprop case. > Note also if either of the max are used more than just in this > simplification, it could increase the lifetime of @0, maybe you need > to add :s to the max expressions. Note :s will be ineffective since the result is "simple", I think it is OK to do this simplification even when the max are not dead after the transform. > > Thanks, > Andrew > > > > > Tests are also included to be added to the testsuite. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR tree-optimization/94290 > > > > gcc/ChangeLog: > > > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. > > * match.pd (x <= 0 ? -x : 0): New Simplification. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.c-torture/execute/pr94290-1.c: New test. > > * gcc.dg/pr94290-2.c: New test. > > * gcc.dg/pr94290.c: New test. > > --- > > gcc/match.pd | 15 ++++++ > > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > > 4 files changed, 92 insertions(+) > > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 45aefd96688..55ca79d7ac9 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7848,3 +7848,18 @@ and, > > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > (bit_and @0 @1) > > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > + > > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > +(simplify > > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > +(simplify > > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > +(simplify > > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > + (max (negate @0) @1)) > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > new file mode 100644 > > index 00000000000..93b80d569aa > > --- /dev/null > > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > @@ -0,0 +1,16 @@ > > +/* PR tree-optimization/94290 */ > > + > > +#include "../../gcc.dg/pr94290.c" > > + > > +int main() { > > + > > + if (foo(0) != 0 > > + || foo(-42) != 42 > > + || foo(42) != 42 > > + || baz(-10) != 10 > > + || baz(-10) != 10) { > > + __builtin_abort(); > > + } > > + > > + return 0; > > +} > > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c > > new file mode 100644 > > index 00000000000..ea6e55755f5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > @@ -0,0 +1,15 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > +/* Form from PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return x <= 0 ? -x : 0; > > +} > > + > > +/* Changed order. */ > > +__attribute__((noipa)) unsigned int bar(int x) { > > + return 0 >= x ? -x : 0; > > +} > > + > > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c > > new file mode 100644 > > index 00000000000..47617c36c02 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > @@ -0,0 +1,46 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > + > > +/* Same form as PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Signed function. */ > > +__attribute__((noipa)) int bar(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Commutative property. */ > > +__attribute__((noipa)) unsigned int baz(int x) { > > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* Flipped order for max expressions. */ > > +__attribute__((noipa)) unsigned int quux(int x) { > > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int waldo(int x) { > > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int fred(int x) { > > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) unsigned int goo(int x) { > > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) int qux(int x) { > > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > -- > > 2.31.1 > >
On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > This patch is intended to fix a missed optimization in match.pd. It > optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > > You could use :c for the commutative property instead and that should > simplify things. > That is: > > (simplify > (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > (abs @0)) > > Also since integer_zerop works on vectors, it seems like you should > add a testcase or two for the vector case. > Also would be useful if you write a testcase that uses different > statements rather than one big one so it gets exercised in the > forwprop case. > Note also if either of the max are used more than just in this > simplification, it could increase the lifetime of @0, maybe you need > to add :s to the max expressions. > > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? Thanks -Sam > Thanks, > Andrew > > > > > Tests are also included to be added to the testsuite. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR tree-optimization/94290 > > > > gcc/ChangeLog: > > > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > > * match.pd (x <= 0 ? -x : 0): New Simplification. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.c-torture/execute/pr94290-1.c: New test. > > * gcc.dg/pr94290-2.c: New test. > > * gcc.dg/pr94290.c: New test. > > --- > > gcc/match.pd | 15 ++++++ > > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > > 4 files changed, 92 insertions(+) > > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 45aefd96688..55ca79d7ac9 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7848,3 +7848,18 @@ and, > > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > (bit_and @0 @1) > > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > + > > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > +(simplify > > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > +(simplify > > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > +(simplify > > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > + (max (negate @0) @1)) > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > new file mode 100644 > > index 00000000000..93b80d569aa > > --- /dev/null > > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > @@ -0,0 +1,16 @@ > > +/* PR tree-optimization/94290 */ > > + > > +#include "../../gcc.dg/pr94290.c" > > + > > +int main() { > > + > > + if (foo(0) != 0 > > + || foo(-42) != 42 > > + || foo(42) != 42 > > + || baz(-10) != 10 > > + || baz(-10) != 10) { > > + __builtin_abort(); > > + } > > + > > + return 0; > > +} > > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > > new file mode 100644 > > index 00000000000..ea6e55755f5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > @@ -0,0 +1,15 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > +/* Form from PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return x <= 0 ? -x : 0; > > +} > > + > > +/* Changed order. */ > > +__attribute__((noipa)) unsigned int bar(int x) { > > + return 0 >= x ? -x : 0; > > +} > > + > > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > > new file mode 100644 > > index 00000000000..47617c36c02 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > @@ -0,0 +1,46 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > + > > +/* Same form as PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Signed function. */ > > +__attribute__((noipa)) int bar(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Commutative property. */ > > +__attribute__((noipa)) unsigned int baz(int x) { > > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* Flipped order for max expressions. */ > > +__attribute__((noipa)) unsigned int quux(int x) { > > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int waldo(int x) { > > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int fred(int x) { > > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) unsigned int goo(int x) { > > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) int qux(int x) { > > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > -- > > 2.31.1 > > > >
On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> <gcc-patches@gcc.gnu.org> wrote: >> > >> > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. >> >> You could use :c for the commutative property instead and that should >> simplify things. >> That is: >> >> (simplify >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> (abs @0)) >> >> Also since integer_zerop works on vectors, it seems like you should >> add a testcase or two for the vector case. >> Also would be useful if you write a testcase that uses different >> statements rather than one big one so it gets exercised in the >> forwprop case. >> Note also if either of the max are used more than just in this >> simplification, it could increase the lifetime of @0, maybe you need >> to add :s to the max expressions. >> > > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? Yes this should produce the pattern at forwprop1 time (with the C++ front-end, the C front-end does not support vector selects): typedef int __attribute__((vector_size(4*sizeof(int)))) vint; vint foo(vint x) { vint t = (x >= 0 ? x : 0) ; vint xx = -x; vint t1 = (xx >= 0 ? xx : 0); return t + t1; } int foo(int x) { int t = (x >= 0 ? x : 0) ; int xx = -x; int t1 = (xx >= 0 ? xx : 0); return t + t1; } Thanks, Andrew Pinski > > Thanks > -Sam > >> >> Thanks, >> Andrew >> >> > >> > Tests are also included to be added to the testsuite. >> > >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> > >> > PR tree-optimization/94290 >> > >> > gcc/ChangeLog: >> > >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> > >> > gcc/testsuite/ChangeLog: >> > >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> > * gcc.dg/pr94290-2.c: New test. >> > * gcc.dg/pr94290.c: New test. >> > --- >> > gcc/match.pd | 15 ++++++ >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ >> > 4 files changed, 92 insertions(+) >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> > >> > diff --git a/gcc/match.pd b/gcc/match.pd >> > index 45aefd96688..55ca79d7ac9 100644 >> > --- a/gcc/match.pd >> > +++ b/gcc/match.pd >> > @@ -7848,3 +7848,18 @@ and, >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> > (bit_and @0 @1) >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> > + >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> > +(simplify >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> > + (abs @0)) >> > + >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> > +(simplify >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> > + (abs @0)) >> > + >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> > +(simplify >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> > + (max (negate @0) @1)) >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > new file mode 100644 >> > index 00000000000..93b80d569aa >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > @@ -0,0 +1,16 @@ >> > +/* PR tree-optimization/94290 */ >> > + >> > +#include "../../gcc.dg/pr94290.c" >> > + >> > +int main() { >> > + >> > + if (foo(0) != 0 >> > + || foo(-42) != 42 >> > + || foo(42) != 42 >> > + || baz(-10) != 10 >> > + || baz(-10) != 10) { >> > + __builtin_abort(); >> > + } >> > + >> > + return 0; >> > +} >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c >> > new file mode 100644 >> > index 00000000000..ea6e55755f5 >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> > @@ -0,0 +1,15 @@ >> > +/* PR tree-optimization/94290 */ >> > +/* { dg-do compile } */ >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> > + >> > +/* Form from PR. */ >> > +__attribute__((noipa)) unsigned int foo(int x) { >> > + return x <= 0 ? -x : 0; >> > +} >> > + >> > +/* Changed order. */ >> > +__attribute__((noipa)) unsigned int bar(int x) { >> > + return 0 >= x ? -x : 0; >> > +} >> > + >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c >> > new file mode 100644 >> > index 00000000000..47617c36c02 >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> > @@ -0,0 +1,46 @@ >> > +/* PR tree-optimization/94290 */ >> > +/* { dg-do compile } */ >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> > + >> > + >> > +/* Same form as PR. */ >> > +__attribute__((noipa)) unsigned int foo(int x) { >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> > +} >> > + >> > +/* Signed function. */ >> > +__attribute__((noipa)) int bar(int x) { >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> > +} >> > + >> > +/* Commutative property. */ >> > +__attribute__((noipa)) unsigned int baz(int x) { >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> > +} >> > + >> > +/* Flipped order for max expressions. */ >> > +__attribute__((noipa)) unsigned int quux(int x) { >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> > +} >> > + >> > +/* Not zero so should not optimize. */ >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> > +} >> > + >> > +/* Not zero so should not optimize. */ >> > +__attribute__((noipa)) unsigned int fred(int x) { >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> > +} >> > + >> > +/* Incorrect pattern. */ >> > +__attribute__((noipa)) unsigned int goo(int x) { >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> > +} >> > + >> > +/* Incorrect pattern. */ >> > +__attribute__((noipa)) int qux(int x) { >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> > +} >> > + >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ >> > >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> > -- >> > 2.31.1 >> > >>
On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> > >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> <gcc-patches@gcc.gnu.org> wrote: > >> > > >> > This patch is intended to fix a missed optimization in match.pd. It > optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > >> > >> You could use :c for the commutative property instead and that should > >> simplify things. > >> That is: > >> > >> (simplify > >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> (abs @0)) > >> > >> Also since integer_zerop works on vectors, it seems like you should > >> add a testcase or two for the vector case. > >> Also would be useful if you write a testcase that uses different > >> statements rather than one big one so it gets exercised in the > >> forwprop case. > >> Note also if either of the max are used more than just in this > >> simplification, it could increase the lifetime of @0, maybe you need > >> to add :s to the max expressions. > >> > > > > Thanks for the feedback. I'm not quite sure what a vector test case > would look like for this. Could I get some guidance on that? > > Yes this should produce the pattern at forwprop1 time (with the C++ > front-end, the C front-end does not support vector selects): > typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > > vint foo(vint x) { > vint t = (x >= 0 ? x : 0) ; > vint xx = -x; > vint t1 = (xx >= 0 ? xx : 0); > return t + t1; > } > > int foo(int x) { > int t = (x >= 0 ? x : 0) ; > int xx = -x; > int t1 = (xx >= 0 ? xx : 0); > return t + t1; > } > > Thanks, > Andrew Pinski > > Thanks for the help. I'm still having trouble with the vector test, though. When I try to compile, I get an error saying "used vector type where scalar is required", referring to the max expressions. How do I use the max expression with two vectors as the inputs? Thanks -Sam > > > > > Thanks > > -Sam > > > >> > >> Thanks, > >> Andrew > >> > >> > > >> > Tests are also included to be added to the testsuite. > >> > > >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> > > >> > PR tree-optimization/94290 > >> > > >> > gcc/ChangeLog: > >> > > >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> > > >> > gcc/testsuite/ChangeLog: > >> > > >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> > * gcc.dg/pr94290-2.c: New test. > >> > * gcc.dg/pr94290.c: New test. > >> > --- > >> > gcc/match.pd | 15 ++++++ > >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > +++++++++++++++++++ > >> > 4 files changed, 92 insertions(+) > >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> > > >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> > index 45aefd96688..55ca79d7ac9 100644 > >> > --- a/gcc/match.pd > >> > +++ b/gcc/match.pd > >> > @@ -7848,3 +7848,18 @@ and, > >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> > (bit_and @0 @1) > >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> > + > >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> > +(simplify > >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> > + (abs @0)) > >> > + > >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> > +(simplify > >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> > + (abs @0)) > >> > + > >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> > +(simplify > >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> > + (max (negate @0) @1)) > >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > new file mode 100644 > >> > index 00000000000..93b80d569aa > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > @@ -0,0 +1,16 @@ > >> > +/* PR tree-optimization/94290 */ > >> > + > >> > +#include "../../gcc.dg/pr94290.c" > >> > + > >> > +int main() { > >> > + > >> > + if (foo(0) != 0 > >> > + || foo(-42) != 42 > >> > + || foo(42) != 42 > >> > + || baz(-10) != 10 > >> > + || baz(-10) != 10) { > >> > + __builtin_abort(); > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > >> > new file mode 100644 > >> > index 00000000000..ea6e55755f5 > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> > @@ -0,0 +1,15 @@ > >> > +/* PR tree-optimization/94290 */ > >> > +/* { dg-do compile } */ > >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> > + > >> > +/* Form from PR. */ > >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> > + return x <= 0 ? -x : 0; > >> > +} > >> > + > >> > +/* Changed order. */ > >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> > + return 0 >= x ? -x : 0; > >> > +} > >> > + > >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > >> > new file mode 100644 > >> > index 00000000000..47617c36c02 > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> > @@ -0,0 +1,46 @@ > >> > +/* PR tree-optimization/94290 */ > >> > +/* { dg-do compile } */ > >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> > + > >> > + > >> > +/* Same form as PR. */ > >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Signed function. */ > >> > +__attribute__((noipa)) int bar(int x) { > >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Commutative property. */ > >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> > +} > >> > + > >> > +/* Flipped order for max expressions. */ > >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> > +} > >> > + > >> > +/* Not zero so should not optimize. */ > >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> > +} > >> > + > >> > +/* Not zero so should not optimize. */ > >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> > +} > >> > + > >> > +/* Incorrect pattern. */ > >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Incorrect pattern. */ > >> > +__attribute__((noipa)) int qux(int x) { > >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> > +} > >> > + > >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > >> > > >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> > -- > >> > 2.31.1 > >> > > >> > >
On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > > > > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: >> > >> > >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> >> <gcc-patches@gcc.gnu.org> wrote: >> >> > >> >> > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. >> >> >> >> You could use :c for the commutative property instead and that should >> >> simplify things. >> >> That is: >> >> >> >> (simplify >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> (abs @0)) >> >> >> >> Also since integer_zerop works on vectors, it seems like you should >> >> add a testcase or two for the vector case. >> >> Also would be useful if you write a testcase that uses different >> >> statements rather than one big one so it gets exercised in the >> >> forwprop case. >> >> Note also if either of the max are used more than just in this >> >> simplification, it could increase the lifetime of @0, maybe you need >> >> to add :s to the max expressions. >> >> >> > >> > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? >> >> Yes this should produce the pattern at forwprop1 time (with the C++ >> front-end, the C front-end does not support vector selects): >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; >> >> vint foo(vint x) { >> vint t = (x >= 0 ? x : 0) ; >> vint xx = -x; >> vint t1 = (xx >= 0 ? xx : 0); >> return t + t1; >> } >> >> int foo(int x) { >> int t = (x >= 0 ? x : 0) ; >> int xx = -x; >> int t1 = (xx >= 0 ? xx : 0); >> return t + t1; >> } >> >> Thanks, >> Andrew Pinski >> > > Thanks for the help. I'm still having trouble with the vector test, though. When I try to compile, I get an error saying "used vector type where scalar is required", referring to the max expressions. How do I use the max expression with two vectors as the inputs? As I mentioned it only works with the C++ front-end :). The C front-end does not support ?: with vectors types. Thanks, Andrew Pinski > > Thanks > -Sam >> >> >> > >> > Thanks >> > -Sam >> > >> >> >> >> Thanks, >> >> Andrew >> >> >> >> > >> >> > Tests are also included to be added to the testsuite. >> >> > >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> > >> >> > PR tree-optimization/94290 >> >> > >> >> > gcc/ChangeLog: >> >> > >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> >> > >> >> > gcc/testsuite/ChangeLog: >> >> > >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> >> > * gcc.dg/pr94290-2.c: New test. >> >> > * gcc.dg/pr94290.c: New test. >> >> > --- >> >> > gcc/match.pd | 15 ++++++ >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ >> >> > 4 files changed, 92 insertions(+) >> >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> >> > >> >> > diff --git a/gcc/match.pd b/gcc/match.pd >> >> > index 45aefd96688..55ca79d7ac9 100644 >> >> > --- a/gcc/match.pd >> >> > +++ b/gcc/match.pd >> >> > @@ -7848,3 +7848,18 @@ and, >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> >> > (bit_and @0 @1) >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> >> > + >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> >> > +(simplify >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> > + (abs @0)) >> >> > + >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> >> > +(simplify >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> >> > + (abs @0)) >> >> > + >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> >> > +(simplify >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> >> > + (max (negate @0) @1)) >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > new file mode 100644 >> >> > index 00000000000..93b80d569aa >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > @@ -0,0 +1,16 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > + >> >> > +#include "../../gcc.dg/pr94290.c" >> >> > + >> >> > +int main() { >> >> > + >> >> > + if (foo(0) != 0 >> >> > + || foo(-42) != 42 >> >> > + || foo(42) != 42 >> >> > + || baz(-10) != 10 >> >> > + || baz(-10) != 10) { >> >> > + __builtin_abort(); >> >> > + } >> >> > + >> >> > + return 0; >> >> > +} >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> > new file mode 100644 >> >> > index 00000000000..ea6e55755f5 >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> > @@ -0,0 +1,15 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > +/* { dg-do compile } */ >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> > + >> >> > +/* Form from PR. */ >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> > + return x <= 0 ? -x : 0; >> >> > +} >> >> > + >> >> > +/* Changed order. */ >> >> > +__attribute__((noipa)) unsigned int bar(int x) { >> >> > + return 0 >= x ? -x : 0; >> >> > +} >> >> > + >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c >> >> > new file mode 100644 >> >> > index 00000000000..47617c36c02 >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> >> > @@ -0,0 +1,46 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > +/* { dg-do compile } */ >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> > + >> >> > + >> >> > +/* Same form as PR. */ >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Signed function. */ >> >> > +__attribute__((noipa)) int bar(int x) { >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Commutative property. */ >> >> > +__attribute__((noipa)) unsigned int baz(int x) { >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> >> > +} >> >> > + >> >> > +/* Flipped order for max expressions. */ >> >> > +__attribute__((noipa)) unsigned int quux(int x) { >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Not zero so should not optimize. */ >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> >> > +} >> >> > + >> >> > +/* Not zero so should not optimize. */ >> >> > +__attribute__((noipa)) unsigned int fred(int x) { >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> >> > +} >> >> > + >> >> > +/* Incorrect pattern. */ >> >> > +__attribute__((noipa)) unsigned int goo(int x) { >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Incorrect pattern. */ >> >> > +__attribute__((noipa)) int qux(int x) { >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> >> > +} >> >> > + >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ >> >> > >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> >> > -- >> >> > 2.31.1 >> >> > >> >> >>
Here's an updated version of the patch. Thanks -Sam On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > > > > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> > >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > >> > > >> > > >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> > wrote: > >> >> > >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> >> <gcc-patches@gcc.gnu.org> wrote: > >> >> > > >> >> > This patch is intended to fix a missed optimization in match.pd. > It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > >> >> > >> >> You could use :c for the commutative property instead and that should > >> >> simplify things. > >> >> That is: > >> >> > >> >> (simplify > >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> (abs @0)) > >> >> > >> >> Also since integer_zerop works on vectors, it seems like you should > >> >> add a testcase or two for the vector case. > >> >> Also would be useful if you write a testcase that uses different > >> >> statements rather than one big one so it gets exercised in the > >> >> forwprop case. > >> >> Note also if either of the max are used more than just in this > >> >> simplification, it could increase the lifetime of @0, maybe you need > >> >> to add :s to the max expressions. > >> >> > >> > > >> > Thanks for the feedback. I'm not quite sure what a vector test case > would look like for this. Could I get some guidance on that? > >> > >> Yes this should produce the pattern at forwprop1 time (with the C++ > >> front-end, the C front-end does not support vector selects): > >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > >> > >> vint foo(vint x) { > >> vint t = (x >= 0 ? x : 0) ; > >> vint xx = -x; > >> vint t1 = (xx >= 0 ? xx : 0); > >> return t + t1; > >> } > >> > >> int foo(int x) { > >> int t = (x >= 0 ? x : 0) ; > >> int xx = -x; > >> int t1 = (xx >= 0 ? xx : 0); > >> return t + t1; > >> } > >> > >> Thanks, > >> Andrew Pinski > >> > > > > Thanks for the help. I'm still having trouble with the vector test, > though. When I try to compile, I get an error saying "used vector type > where scalar is required", referring to the max expressions. How do I use > the max expression with two vectors as the inputs? > > As I mentioned it only works with the C++ front-end :). The C > front-end does not support ?: with vectors types. > > Thanks, > Andrew Pinski > > > > > Thanks > > -Sam > >> > >> > >> > > >> > Thanks > >> > -Sam > >> > > >> >> > >> >> Thanks, > >> >> Andrew > >> >> > >> >> > > >> >> > Tests are also included to be added to the testsuite. > >> >> > > >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> >> > > >> >> > PR tree-optimization/94290 > >> >> > > >> >> > gcc/ChangeLog: > >> >> > > >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> >> > > >> >> > gcc/testsuite/ChangeLog: > >> >> > > >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> >> > * gcc.dg/pr94290-2.c: New test. > >> >> > * gcc.dg/pr94290.c: New test. > >> >> > --- > >> >> > gcc/match.pd | 15 ++++++ > >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > +++++++++++++++++++ > >> >> > 4 files changed, 92 insertions(+) > >> >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> >> > > >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> >> > index 45aefd96688..55ca79d7ac9 100644 > >> >> > --- a/gcc/match.pd > >> >> > +++ b/gcc/match.pd > >> >> > @@ -7848,3 +7848,18 @@ and, > >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> >> > (bit_and @0 @1) > >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> >> > + > >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> >> > +(simplify > >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> > + (abs @0)) > >> >> > + > >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> >> > +(simplify > >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> >> > + (abs @0)) > >> >> > + > >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> >> > +(simplify > >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> >> > + (max (negate @0) @1)) > >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > new file mode 100644 > >> >> > index 00000000000..93b80d569aa > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > @@ -0,0 +1,16 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > + > >> >> > +#include "../../gcc.dg/pr94290.c" > >> >> > + > >> >> > +int main() { > >> >> > + > >> >> > + if (foo(0) != 0 > >> >> > + || foo(-42) != 42 > >> >> > + || foo(42) != 42 > >> >> > + || baz(-10) != 10 > >> >> > + || baz(-10) != 10) { > >> >> > + __builtin_abort(); > >> >> > + } > >> >> > + > >> >> > + return 0; > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > new file mode 100644 > >> >> > index 00000000000..ea6e55755f5 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > @@ -0,0 +1,15 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> > + > >> >> > +/* Form from PR. */ > >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> > + return x <= 0 ? -x : 0; > >> >> > +} > >> >> > + > >> >> > +/* Changed order. */ > >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> >> > + return 0 >= x ? -x : 0; > >> >> > +} > >> >> > + > >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } > } */ > >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > >> >> > new file mode 100644 > >> >> > index 00000000000..47617c36c02 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> >> > @@ -0,0 +1,46 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> > + > >> >> > + > >> >> > +/* Same form as PR. */ > >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Signed function. */ > >> >> > +__attribute__((noipa)) int bar(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Commutative property. */ > >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Flipped order for max expressions. */ > >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Not zero so should not optimize. */ > >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> >> > +} > >> >> > + > >> >> > +/* Not zero so should not optimize. */ > >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> >> > +} > >> >> > + > >> >> > +/* Incorrect pattern. */ > >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Incorrect pattern. */ > >> >> > +__attribute__((noipa)) int qux(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> >> > +} > >> >> > + > >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } > } */ > >> >> > > >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> >> > -- > >> >> > 2.31.1 > >> >> > > >> >> > >> > >
Just realized I had mixed up the 9 and the 2 when labelling the patch. This patch is referring to pr94920 not pr94290. Attached is a fixed patch file. Sorry for any confusion. On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > Here's an updated version of the patch. > > Thanks > -Sam > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: >> > >> > >> > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> >> wrote: >> >> >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: >> >> > >> >> > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> >> wrote: >> >> >> >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> >> >> <gcc-patches@gcc.gnu.org> wrote: >> >> >> > >> >> >> > This patch is intended to fix a missed optimization in match.pd. >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to >> write a second simplification in match.pd to handle the commutative >> property as the match was not ocurring otherwise. Additionally, the pattern >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the >> other simplification rule. >> >> >> >> >> >> You could use :c for the commutative property instead and that >> should >> >> >> simplify things. >> >> >> That is: >> >> >> >> >> >> (simplify >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> >> (abs @0)) >> >> >> >> >> >> Also since integer_zerop works on vectors, it seems like you should >> >> >> add a testcase or two for the vector case. >> >> >> Also would be useful if you write a testcase that uses different >> >> >> statements rather than one big one so it gets exercised in the >> >> >> forwprop case. >> >> >> Note also if either of the max are used more than just in this >> >> >> simplification, it could increase the lifetime of @0, maybe you need >> >> >> to add :s to the max expressions. >> >> >> >> >> > >> >> > Thanks for the feedback. I'm not quite sure what a vector test case >> would look like for this. Could I get some guidance on that? >> >> >> >> Yes this should produce the pattern at forwprop1 time (with the C++ >> >> front-end, the C front-end does not support vector selects): >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; >> >> >> >> vint foo(vint x) { >> >> vint t = (x >= 0 ? x : 0) ; >> >> vint xx = -x; >> >> vint t1 = (xx >= 0 ? xx : 0); >> >> return t + t1; >> >> } >> >> >> >> int foo(int x) { >> >> int t = (x >= 0 ? x : 0) ; >> >> int xx = -x; >> >> int t1 = (xx >= 0 ? xx : 0); >> >> return t + t1; >> >> } >> >> >> >> Thanks, >> >> Andrew Pinski >> >> >> > >> > Thanks for the help. I'm still having trouble with the vector test, >> though. When I try to compile, I get an error saying "used vector type >> where scalar is required", referring to the max expressions. How do I use >> the max expression with two vectors as the inputs? >> >> As I mentioned it only works with the C++ front-end :). The C >> front-end does not support ?: with vectors types. >> >> Thanks, >> Andrew Pinski >> >> > >> > Thanks >> > -Sam >> >> >> >> >> >> > >> >> > Thanks >> >> > -Sam >> >> > >> >> >> >> >> >> Thanks, >> >> >> Andrew >> >> >> >> >> >> > >> >> >> > Tests are also included to be added to the testsuite. >> >> >> > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> >> > >> >> >> > PR tree-optimization/94290 >> >> >> > >> >> >> > gcc/ChangeLog: >> >> >> > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New >> simplification. >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> >> >> > >> >> >> > gcc/testsuite/ChangeLog: >> >> >> > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> >> >> > * gcc.dg/pr94290-2.c: New test. >> >> >> > * gcc.dg/pr94290.c: New test. >> >> >> > --- >> >> >> > gcc/match.pd | 15 ++++++ >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 >> +++++++++++++++++++ >> >> >> > 4 files changed, 92 insertions(+) >> >> >> > create mode 100644 >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> >> >> > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd >> >> >> > index 45aefd96688..55ca79d7ac9 100644 >> >> >> > --- a/gcc/match.pd >> >> >> > +++ b/gcc/match.pd >> >> >> > @@ -7848,3 +7848,18 @@ and, >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> >> >> > (bit_and @0 @1) >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> >> >> > + >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> >> >> > +(simplify >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> >> > + (abs @0)) >> >> >> > + >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> >> >> > +(simplify >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> >> >> > + (abs @0)) >> >> >> > + >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> >> >> > +(simplify >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> >> >> > + (max (negate @0) @1)) >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..93b80d569aa >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > @@ -0,0 +1,16 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > + >> >> >> > +#include "../../gcc.dg/pr94290.c" >> >> >> > + >> >> >> > +int main() { >> >> >> > + >> >> >> > + if (foo(0) != 0 >> >> >> > + || foo(-42) != 42 >> >> >> > + || foo(42) != 42 >> >> >> > + || baz(-10) != 10 >> >> >> > + || baz(-10) != 10) { >> >> >> > + __builtin_abort(); >> >> >> > + } >> >> >> > + >> >> >> > + return 0; >> >> >> > +} >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c >> b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..ea6e55755f5 >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > @@ -0,0 +1,15 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > +/* { dg-do compile } */ >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> >> > + >> >> >> > +/* Form from PR. */ >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> >> > + return x <= 0 ? -x : 0; >> >> >> > +} >> >> >> > + >> >> >> > +/* Changed order. */ >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { >> >> >> > + return 0 >= x ? -x : 0; >> >> >> > +} >> >> >> > + >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } >> } */ >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c >> b/gcc/testsuite/gcc.dg/pr94290.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..47617c36c02 >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> >> >> > @@ -0,0 +1,46 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > +/* { dg-do compile } */ >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> >> > + >> >> >> > + >> >> >> > +/* Same form as PR. */ >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Signed function. */ >> >> >> > +__attribute__((noipa)) int bar(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Commutative property. */ >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Flipped order for max expressions. */ >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Not zero so should not optimize. */ >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> >> >> > +} >> >> >> > + >> >> >> > +/* Not zero so should not optimize. */ >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> >> >> > +} >> >> >> > + >> >> >> > +/* Incorrect pattern. */ >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Incorrect pattern. */ >> >> >> > +__attribute__((noipa)) int qux(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } >> } */ >> >> >> > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> >> >> > -- >> >> >> > 2.31.1 >> >> >> > >> >> >> >> >> >> >>
On Mon, Jul 18, 2022 at 7:31 PM Sam Feifer via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Just realized I had mixed up the 9 and the 2 when labelling the patch. This > patch is referring to pr94920 not pr94290. Attached is a fixed patch file. > Sorry for any confusion. Can you put the patterns somewhere related? The abs producing one seems to fit around line 323, the max producing before line 3415? +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) you can re-use the existing (negate @0) in the result by doing +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1) + (max @2 @1)) OK with those changes. Thanks, Richard. > On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > Here's an updated version of the patch. > > > > Thanks > > -Sam > > > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > >> > > >> > > >> > > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> > >> wrote: > >> >> > >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > >> >> > > >> >> > > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> > >> wrote: > >> >> >> > >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> >> >> <gcc-patches@gcc.gnu.org> wrote: > >> >> >> > > >> >> >> > This patch is intended to fix a missed optimization in match.pd. > >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > >> write a second simplification in match.pd to handle the commutative > >> property as the match was not ocurring otherwise. Additionally, the pattern > >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > >> other simplification rule. > >> >> >> > >> >> >> You could use :c for the commutative property instead and that > >> should > >> >> >> simplify things. > >> >> >> That is: > >> >> >> > >> >> >> (simplify > >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> >> (abs @0)) > >> >> >> > >> >> >> Also since integer_zerop works on vectors, it seems like you should > >> >> >> add a testcase or two for the vector case. > >> >> >> Also would be useful if you write a testcase that uses different > >> >> >> statements rather than one big one so it gets exercised in the > >> >> >> forwprop case. > >> >> >> Note also if either of the max are used more than just in this > >> >> >> simplification, it could increase the lifetime of @0, maybe you need > >> >> >> to add :s to the max expressions. > >> >> >> > >> >> > > >> >> > Thanks for the feedback. I'm not quite sure what a vector test case > >> would look like for this. Could I get some guidance on that? > >> >> > >> >> Yes this should produce the pattern at forwprop1 time (with the C++ > >> >> front-end, the C front-end does not support vector selects): > >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > >> >> > >> >> vint foo(vint x) { > >> >> vint t = (x >= 0 ? x : 0) ; > >> >> vint xx = -x; > >> >> vint t1 = (xx >= 0 ? xx : 0); > >> >> return t + t1; > >> >> } > >> >> > >> >> int foo(int x) { > >> >> int t = (x >= 0 ? x : 0) ; > >> >> int xx = -x; > >> >> int t1 = (xx >= 0 ? xx : 0); > >> >> return t + t1; > >> >> } > >> >> > >> >> Thanks, > >> >> Andrew Pinski > >> >> > >> > > >> > Thanks for the help. I'm still having trouble with the vector test, > >> though. When I try to compile, I get an error saying "used vector type > >> where scalar is required", referring to the max expressions. How do I use > >> the max expression with two vectors as the inputs? > >> > >> As I mentioned it only works with the C++ front-end :). The C > >> front-end does not support ?: with vectors types. > >> > >> Thanks, > >> Andrew Pinski > >> > >> > > >> > Thanks > >> > -Sam > >> >> > >> >> > >> >> > > >> >> > Thanks > >> >> > -Sam > >> >> > > >> >> >> > >> >> >> Thanks, > >> >> >> Andrew > >> >> >> > >> >> >> > > >> >> >> > Tests are also included to be added to the testsuite. > >> >> >> > > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> >> >> > > >> >> >> > PR tree-optimization/94290 > >> >> >> > > >> >> >> > gcc/ChangeLog: > >> >> >> > > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > >> simplification. > >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> >> >> > > >> >> >> > gcc/testsuite/ChangeLog: > >> >> >> > > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> >> >> > * gcc.dg/pr94290-2.c: New test. > >> >> >> > * gcc.dg/pr94290.c: New test. > >> >> >> > --- > >> >> >> > gcc/match.pd | 15 ++++++ > >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > >> +++++++++++++++++++ > >> >> >> > 4 files changed, 92 insertions(+) > >> >> >> > create mode 100644 > >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> >> >> > index 45aefd96688..55ca79d7ac9 100644 > >> >> >> > --- a/gcc/match.pd > >> >> >> > +++ b/gcc/match.pd > >> >> >> > @@ -7848,3 +7848,18 @@ and, > >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> >> >> > (bit_and @0 @1) > >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> >> >> > + > >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> >> >> > +(simplify > >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> >> > + (abs @0)) > >> >> >> > + > >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> >> >> > +(simplify > >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> >> >> > + (abs @0)) > >> >> >> > + > >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> >> >> > +(simplify > >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> >> >> > + (max (negate @0) @1)) > >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..93b80d569aa > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > @@ -0,0 +1,16 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > + > >> >> >> > +#include "../../gcc.dg/pr94290.c" > >> >> >> > + > >> >> >> > +int main() { > >> >> >> > + > >> >> >> > + if (foo(0) != 0 > >> >> >> > + || foo(-42) != 42 > >> >> >> > + || foo(42) != 42 > >> >> >> > + || baz(-10) != 10 > >> >> >> > + || baz(-10) != 10) { > >> >> >> > + __builtin_abort(); > >> >> >> > + } > >> >> >> > + > >> >> >> > + return 0; > >> >> >> > +} > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > >> b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..ea6e55755f5 > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > @@ -0,0 +1,15 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > +/* { dg-do compile } */ > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> >> > + > >> >> >> > +/* Form from PR. */ > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> >> > + return x <= 0 ? -x : 0; > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Changed order. */ > >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> >> >> > + return 0 >= x ? -x : 0; > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } > >> } */ > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > >> b/gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..47617c36c02 > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > @@ -0,0 +1,46 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > +/* { dg-do compile } */ > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> >> > + > >> >> >> > + > >> >> >> > +/* Same form as PR. */ > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Signed function. */ > >> >> >> > +__attribute__((noipa)) int bar(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Commutative property. */ > >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Flipped order for max expressions. */ > >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Not zero so should not optimize. */ > >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Not zero so should not optimize. */ > >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Incorrect pattern. */ > >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Incorrect pattern. */ > >> >> >> > +__attribute__((noipa)) int qux(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } > >> } */ > >> >> >> > > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> >> >> > -- > >> >> >> > 2.31.1 > >> >> >> > > >> >> >> > >> >> > >> > >>
Attached is a patch file with those changes. Thanks -Sam On Tue, Jul 19, 2022 at 2:36 AM Richard Biener <richard.guenther@gmail.com> wrote: > On Mon, Jul 18, 2022 at 7:31 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > Just realized I had mixed up the 9 and the 2 when labelling the patch. > This > > patch is referring to pr94920 not pr94290. Attached is a fixed patch > file. > > Sorry for any confusion. > > Can you put the patterns somewhere related? The abs producing one > seems to fit around line 323, the max producing before line 3415? > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > + (max (negate @0) @1)) > > you can re-use the existing (negate @0) in the result by doing > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1) > + (max @2 @1)) > > OK with those changes. > > Thanks, > Richard. > > > On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > Here's an updated version of the patch. > > > > > > Thanks > > > -Sam > > > > > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> > wrote: > > > > > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> > wrote: > > >> > > > >> > > > >> > > > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> > > >> wrote: > > >> >> > > >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> > wrote: > > >> >> > > > >> >> > > > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com > > > > >> wrote: > > >> >> >> > > >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > > >> >> >> <gcc-patches@gcc.gnu.org> wrote: > > >> >> >> > > > >> >> >> > This patch is intended to fix a missed optimization in > match.pd. > > >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I > had to > > >> write a second simplification in match.pd to handle the commutative > > >> property as the match was not ocurring otherwise. Additionally, the > pattern > > >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with > the > > >> other simplification rule. > > >> >> >> > > >> >> >> You could use :c for the commutative property instead and that > > >> should > > >> >> >> simplify things. > > >> >> >> That is: > > >> >> >> > > >> >> >> (simplify > > >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) > integer_zerop)) > > >> >> >> (abs @0)) > > >> >> >> > > >> >> >> Also since integer_zerop works on vectors, it seems like you > should > > >> >> >> add a testcase or two for the vector case. > > >> >> >> Also would be useful if you write a testcase that uses different > > >> >> >> statements rather than one big one so it gets exercised in the > > >> >> >> forwprop case. > > >> >> >> Note also if either of the max are used more than just in this > > >> >> >> simplification, it could increase the lifetime of @0, maybe you > need > > >> >> >> to add :s to the max expressions. > > >> >> >> > > >> >> > > > >> >> > Thanks for the feedback. I'm not quite sure what a vector test > case > > >> would look like for this. Could I get some guidance on that? > > >> >> > > >> >> Yes this should produce the pattern at forwprop1 time (with the C++ > > >> >> front-end, the C front-end does not support vector selects): > > >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > > >> >> > > >> >> vint foo(vint x) { > > >> >> vint t = (x >= 0 ? x : 0) ; > > >> >> vint xx = -x; > > >> >> vint t1 = (xx >= 0 ? xx : 0); > > >> >> return t + t1; > > >> >> } > > >> >> > > >> >> int foo(int x) { > > >> >> int t = (x >= 0 ? x : 0) ; > > >> >> int xx = -x; > > >> >> int t1 = (xx >= 0 ? xx : 0); > > >> >> return t + t1; > > >> >> } > > >> >> > > >> >> Thanks, > > >> >> Andrew Pinski > > >> >> > > >> > > > >> > Thanks for the help. I'm still having trouble with the vector test, > > >> though. When I try to compile, I get an error saying "used vector type > > >> where scalar is required", referring to the max expressions. How do I > use > > >> the max expression with two vectors as the inputs? > > >> > > >> As I mentioned it only works with the C++ front-end :). The C > > >> front-end does not support ?: with vectors types. > > >> > > >> Thanks, > > >> Andrew Pinski > > >> > > >> > > > >> > Thanks > > >> > -Sam > > >> >> > > >> >> > > >> >> > > > >> >> > Thanks > > >> >> > -Sam > > >> >> > > > >> >> >> > > >> >> >> Thanks, > > >> >> >> Andrew > > >> >> >> > > >> >> >> > > > >> >> >> > Tests are also included to be added to the testsuite. > > >> >> >> > > > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > >> >> >> > > > >> >> >> > PR tree-optimization/94290 > > >> >> >> > > > >> >> >> > gcc/ChangeLog: > > >> >> >> > > > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > > >> simplification. > > >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > > >> >> >> > > > >> >> >> > gcc/testsuite/ChangeLog: > > >> >> >> > > > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > > >> >> >> > * gcc.dg/pr94290-2.c: New test. > > >> >> >> > * gcc.dg/pr94290.c: New test. > > >> >> >> > --- > > >> >> >> > gcc/match.pd | 15 ++++++ > > >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > > >> +++++++++++++++++++ > > >> >> >> > 4 files changed, 92 insertions(+) > > >> >> >> > create mode 100644 > > >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > > > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > > >> >> >> > index 45aefd96688..55ca79d7ac9 100644 > > >> >> >> > --- a/gcc/match.pd > > >> >> >> > +++ b/gcc/match.pd > > >> >> >> > @@ -7848,3 +7848,18 @@ and, > > >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > >> >> >> > (bit_and @0 @1) > > >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > >> >> >> > + > > >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > >> >> >> > +(simplify > > >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) > integer_zerop)) > > >> >> >> > + (abs @0)) > > >> >> >> > + > > >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > >> >> >> > +(simplify > > >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 > integer_zerop) ) > > >> >> >> > + (abs @0)) > > >> >> >> > + > > >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > >> >> >> > +(simplify > > >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > >> >> >> > + (max (negate @0) @1)) > > >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..93b80d569aa > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > @@ -0,0 +1,16 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > + > > >> >> >> > +#include "../../gcc.dg/pr94290.c" > > >> >> >> > + > > >> >> >> > +int main() { > > >> >> >> > + > > >> >> >> > + if (foo(0) != 0 > > >> >> >> > + || foo(-42) != 42 > > >> >> >> > + || foo(42) != 42 > > >> >> >> > + || baz(-10) != 10 > > >> >> >> > + || baz(-10) != 10) { > > >> >> >> > + __builtin_abort(); > > >> >> >> > + } > > >> >> >> > + > > >> >> >> > + return 0; > > >> >> >> > +} > > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > > >> b/gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..ea6e55755f5 > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > @@ -0,0 +1,15 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > +/* { dg-do compile } */ > > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > >> >> >> > + > > >> >> >> > +/* Form from PR. */ > > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > > >> >> >> > + return x <= 0 ? -x : 0; > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Changed order. */ > > >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > > >> >> >> > + return 0 >= x ? -x : 0; > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 > "optimized" } > > >> } */ > > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > > >> b/gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..47617c36c02 > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > @@ -0,0 +1,46 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > +/* { dg-do compile } */ > > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > >> >> >> > + > > >> >> >> > + > > >> >> >> > +/* Same form as PR. */ > > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Signed function. */ > > >> >> >> > +__attribute__((noipa)) int bar(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Commutative property. */ > > >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > > >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Flipped order for max expressions. */ > > >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > > >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Not zero so should not optimize. */ > > >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > > >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Not zero so should not optimize. */ > > >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > > >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Incorrect pattern. */ > > >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > > >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Incorrect pattern. */ > > >> >> >> > +__attribute__((noipa)) int qux(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 > "optimized" } > > >> } */ > > >> >> >> > > > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > >> >> >> > -- > > >> >> >> > 2.31.1 > > >> >> >> > > > >> >> >> > > >> >> > > >> > > >> > >
diff --git a/gcc/match.pd b/gcc/match.pd index 45aefd96688..55ca79d7ac9 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7848,3 +7848,18 @@ and, (if (TYPE_UNSIGNED (TREE_TYPE (@0))) (bit_and @0 @1) (cond (le @0 @1) @0 (bit_and @0 @1)))))) + +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ +(simplify + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) + (abs @0)) + +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ +(simplify + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) + (abs @0)) + +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c new file mode 100644 index 00000000000..93b80d569aa --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c @@ -0,0 +1,16 @@ +/* PR tree-optimization/94290 */ + +#include "../../gcc.dg/pr94290.c" + +int main() { + + if (foo(0) != 0 + || foo(-42) != 42 + || foo(42) != 42 + || baz(-10) != 10 + || baz(-10) != 10) { + __builtin_abort(); + } + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c new file mode 100644 index 00000000000..ea6e55755f5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94290-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* Form from PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return x <= 0 ? -x : 0; +} + +/* Changed order. */ +__attribute__((noipa)) unsigned int bar(int x) { + return 0 >= x ? -x : 0; +} + +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c new file mode 100644 index 00000000000..47617c36c02 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94290.c @@ -0,0 +1,46 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + + +/* Same form as PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Signed function. */ +__attribute__((noipa)) int bar(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Commutative property. */ +__attribute__((noipa)) unsigned int baz(int x) { + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); +} + +/* Flipped order for max expressions. */ +__attribute__((noipa)) unsigned int quux(int x) { + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int waldo(int x) { + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int fred(int x) { + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) unsigned int goo(int x) { + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) int qux(int x) { + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); +} + +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */