From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) by sourceware.org (Postfix) with ESMTPS id AF5123855005 for ; Mon, 12 Jul 2021 11:46:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AF5123855005 Received: by mail-ej1-x62e.google.com with SMTP id i20so34004737ejw.4 for ; Mon, 12 Jul 2021 04:46:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=DYa4fWNaKlojeCuqGwWepjk8ZQk/koB4kgfUXg/nIV4=; b=CDE3qq+aa610aDsQC65bbNj66BnbaxCLzt01tPVEloMIJofHAMTW3D4OfzOIMz2yDw vpUHRsxQiqNINh97MVWDz/BPrYsdb23JgavOdsObGMqPGZPvuJbOSin0uL1J/3DkXkJ4 DulkTL9DC8+B9/Yz+wI59q/7ykLb2OCsMYZlj/AHTBvvPk+HNLw1N5nRXl03UHR04a7N 50Umd8hahHK2/BvpS5SFYaJB+oGVGgyp6B4JjyY8LBSzxzJyzxhvRQAce/hvkBG6YjeS 1CfdznN2Kt95qbhSdEuG+aHcoP763Id9JrVmDmz/xZlBoRAx5M8IFhJ2EmDqIoWkJ7WY 101A== X-Gm-Message-State: AOAM532VZ9URST4XW+V1lzTTc6OOt86f/+Gwki5R9QLx07BphVV3xu48 HSwWIJ7h/JR3UQ2ceCYipTqXoR+Qve661XILeVU= X-Google-Smtp-Source: ABdhPJyVoUfdhjYHkT3PhGVSWrgwpsMpBcWi+fDVW41DwNQfIT2dRjxBP/XH9rc9PvMnw4umD6sGvVjwwYZ+lpXHySs= X-Received: by 2002:a17:907:9812:: with SMTP id ji18mr53179221ejc.138.1626090403645; Mon, 12 Jul 2021 04:46:43 -0700 (PDT) MIME-Version: 1.0 References: <1625969489-14344-1-git-send-email-apinski@marvell.com> In-Reply-To: <1625969489-14344-1-git-send-email-apinski@marvell.com> From: Richard Biener Date: Mon, 12 Jul 2021 13:46:32 +0200 Message-ID: Subject: Re: [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match To: Andrew Pinski Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jul 2021 11:46:46 -0000 On Sun, Jul 11, 2021 at 4:12 AM apinski--- via Gcc-patches wrote: > > From: Andrew Pinski > > This patch moves the (a-b) CMP 0 ? (a-b) : (b-a) optimization > from fold_cond_expr_with_comparison to match. So I searched and I guess these transforms are produced from /* If we have A op 0 ? A : -A, consider applying the following transformations: A == 0? A : -A same as -A A != 0? A : -A same as A A >= 0? A : -A same as abs (A) A > 0? A : -A same as abs (A) A <= 0? A : -A same as -abs (A) A < 0? A : -A same as -abs (A) None of these transformations work for modes with signed zeros. If A is +/-0, the first two transformations will change the sign of the result (from +0 to -0, or vice versa). The last four will fix the sign of the result, even though the original expressions could be positive or negative, depending on the sign of A. Note that all these transformations are correct if A is NaN, since the two alternatives (A and -A) are also NaNs. */ if (!HONOR_SIGNED_ZEROS (type) && (FLOAT_TYPE_P (TREE_TYPE (arg01)) ? real_zerop (arg01) : integer_zerop (arg01)) && ((TREE_CODE (arg2) == NEGATE_EXPR && operand_equal_p (TREE_OPERAND (arg2, 0), arg1, 0)) /* In the case that A is of the form X-Y, '-A' (arg2) may have already been folded to Y-X, check for that. */ || (TREE_CODE (arg1) == MINUS_EXPR && TREE_CODE (arg2) == MINUS_EXPR && operand_equal_p (TREE_OPERAND (arg1, 0), TREE_OPERAND (arg2, 1), 0) && operand_equal_p (TREE_OPERAND (arg1, 1), TREE_OPERAND (arg2, 0), 0)))) ... I wonder at which point we can remove the code from fold-const.c? Some comments inline below. > OK? Bootstrapped and tested on x86_64-linux-gnu. > > gcc/ChangeLog: > > * match.pd ((A-B) CMP 0 ? (A-B) : (B - A)): > New patterns. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/phi-opt-25.c: New test. > --- > gcc/match.pd | 48 ++++++++++++++++++++-- > gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c | 45 ++++++++++++++++++++ > 2 files changed, 90 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 30680d488ab..aa88381fdcb 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -4040,9 +4040,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > (cnd (logical_inverted_value truth_valued_p@0) @1 @2) > (cnd @0 @2 @1))) > > -/* abs/negative simplifications moved from fold_cond_expr_with_comparison, > - Need to handle (A - B) case as fold_cond_expr_with_comparison does. > - Need to handle UN* comparisons. > +/* abs/negative simplifications moved from fold_cond_expr_with_comparison. > > None of these transformations work for modes with signed > zeros. If A is +/-0, the first two transformations will > @@ -4098,6 +4096,50 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > (convert (negate (absu:utype @0)))) > (negate (abs @0))))) > ) > + > + /* (A - B) == 0 ? (A - B) : (B - A) same as (B - A) */ > + (for cmp (eq uneq) > + (simplify > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus@3 @2 @1)) > + (if (!HONOR_SIGNED_ZEROS (type)) > + @3)) > + (simplify > + (cnd (cmp (minus@0 @1 @2) zerop) integer_zerop (minus@3 @2 @1)) So that makes me think why integer_zerop? 'type' should then be integer and thus never HONOR_SIGNED_ZEROS. Don't we also need the inverted condition case for completeness? > + (if (!HONOR_SIGNED_ZEROS (type)) > + @3)) > + (simplify > + (cnd (cmp @1 @2) integer_zerop (minus@3 @2 @1)) I think this needs to be (cmp:c @1 @2) > + (if (!HONOR_SIGNED_ZEROS (type)) > + @3)) > + ) > + /* (A - B) != 0 ? (A - B) : (B - A) same as (A - B) */ > + (for cmp (ne ltgt) > + (simplify > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1)) > + (if (!HONOR_SIGNED_ZEROS (type)) > + @0)) > + ) > + /* (A - B) >=/> 0 ? (A - B) : (B - A) same as abs (A - B) */ > + (for cmp (ge gt) > + (simplify > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1)) > + (if (!HONOR_SIGNED_ZEROS (type) > + && !TYPE_UNSIGNED (type)) > + (abs @0)))) > + /* (A - B) <=/< 0 ? (A - B) : (B - A) same as -abs (A - B) */ > + (for cmp (le lt) > + (simplify > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1)) > + (if (!HONOR_SIGNED_ZEROS (type) > + && !TYPE_UNSIGNED (type)) > + (if (ANY_INTEGRAL_TYPE_P (type) > + && !TYPE_OVERFLOW_WRAPS (type)) > + (with { > + tree utype = unsigned_type_for (type); > + } > + (convert (negate (absu:utype @0)))) > + (negate (abs @0))))) > + ) > ) > > /* -(type)!A -> (type)A - 1. */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c > new file mode 100644 > index 00000000000..0f0e3170f8d > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c > @@ -0,0 +1,45 @@ > +/* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */ > +int minus1(int a, int b) > +{ > + int c = a - b; > + if (c == 0) c = b - a; > + return c; > +} > +int minus2(int a, int b) > +{ > + int c = a - b; > + if (c != 0) c = b - a; > + return c; > +} > +int minus3(int a, int b) > +{ > + int c = a - b; > + if (c == 0) c = 0; > + else c = b - a; > + return c; > +} > +int minus4(int a, int b) > +{ > + int c; > + if (a == b) c = 0; > + else > + c = b - a; > + return c; > +} > +int abs0(int a, int b) > +{ > + int c = a - b; > + if (c <= 0) c = b - a; > + return c; > +} > +int negabs(int a, int b) > +{ > + int c = a - b; > + if (c >= 0) c = b - a; > + return c; > +} > + > +/* The above should be optimized at phiopt1 except for negabs which has to wait > + until phiopt2 as -abs is not acceptable in early phiopt. */ > +/* { dg-final { scan-tree-dump-times "if" 1 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-not "if" "phiopt2" } } */ > -- > 2.27.0 >