From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 290D43858C55 for ; Thu, 14 Jul 2022 14:09:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 290D43858C55 Received: from mail-pj1-f71.google.com (mail-pj1-f71.google.com [209.85.216.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-397-6EX9N2v0NkKMlhCw9QUVLw-1; Thu, 14 Jul 2022 10:09:39 -0400 X-MC-Unique: 6EX9N2v0NkKMlhCw9QUVLw-1 Received: by mail-pj1-f71.google.com with SMTP id o8-20020a17090ab88800b001ef81869167so3755153pjr.2 for ; Thu, 14 Jul 2022 07:09:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=XjIgJ7bl5g7TXB75CoNhGEHd7LYqM7qDTIK5rkZkimY=; b=2YgQ/PibhNNm7nS0n2tog8zK4nR6IKLumYo8OLoIOAt49jatCmIKXftz8P9SrSRBhG wjAuOtWdev2f3BcJ3VYLXmKztzl3FFEmhlETgd0IFcDQTT7FaKp9szA011JexRRGklz/ aCVNk9fyQsTNOZiH9yDK/rN45bnvJ7foB4ngZOJQf/SRElJLJnwzSMYwOS85qZVtFHHK RHgHP8P1XQahERMUmTdEBUEYjZoKBF64LQTEilTm0iBzRq+n6ZHabHCMhbgxaoN0S852 WJi2XwrcCJ/vLPGK88OULfuXWaqswFCZw84qaS1m8yso9efsNhbcpmohJJ5l+WU5EZwI 7KKQ== X-Gm-Message-State: AJIora9dI2jvvMEJ59xHzmjfE6+MZonFr2B8QNqEVPrY/hnjJSiTHsox jxQOhYW+n/90h+vVEzAYAo/sK20ljwk5sC5sZrTr2FVJ8KLO/Kf3aQeZaYLGPmi9LoV7BydlC3Y t5TmcLFAyQihqXg6KIl2NAJVURDXH7AEfvA== X-Received: by 2002:a17:902:f691:b0:16c:4043:f6a2 with SMTP id l17-20020a170902f69100b0016c4043f6a2mr8479178plg.72.1657807778441; Thu, 14 Jul 2022 07:09:38 -0700 (PDT) X-Google-Smtp-Source: AGRyM1uWeNeQPWEp2fIbh7Q0T+UdEOW+TO30+UzWd21g69z5gjzSXreHN+BwBa48IjH/1dB8mXNCsWdopdKSWVWxtJU= X-Received: by 2002:a17:902:f691:b0:16c:4043:f6a2 with SMTP id l17-20020a170902f69100b0016c4043f6a2mr8479157plg.72.1657807777998; Thu, 14 Jul 2022 07:09:37 -0700 (PDT) MIME-Version: 1.0 References: <20220713192420.3126654-1-sfeifer@redhat.com> In-Reply-To: From: Sam Feifer Date: Thu, 14 Jul 2022 10:09:27 -0400 Message-ID: Subject: Re: [PATCH] match.pd: Add new abs pattern [PR94290] To: Andrew Pinski Cc: GCC Patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jul 2022 14:09:44 -0000 On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski wrote: > On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > 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 > > > >