From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 5B0153858D1E for ; Thu, 3 Aug 2023 08:07:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5B0153858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-wr1-x42e.google.com with SMTP id ffacd0b85a97d-31783d02093so607136f8f.0 for ; Thu, 03 Aug 2023 01:07:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1691050061; x=1691654861; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=uPOhncDHb3UGNlSGuoarFvMLM7D6Id86W7w4DtvEWCw=; b=CMtUXuwqKijW3iCOXhNtUX4HOsTOHEFkkLgeWkSvIc6oboqWEIpTUFi5VCbrswZwAP 7HoP6UM5gbIGerjkudeTQ98kr3iyOFsvkvw/PT3/hkps/w6qjCa/ZxXG8oPSoZmBSGUV aMQYNJ+uVbhcfs/6zpIZ03ov/z4H7755wZauJAjOy4Oxxd8elP+t+uXzQDgdtgvBkc8L Scby1/GZIv0rstKhj7FF4JeOqJgMwYTMTwIkPKfd9AT1rN78Uz51GPzH6dlUB/3Ntc19 u+u1obyHva4O4uTUm+jbVdixYwZ4CR9rVUusA1eox2idcJ7WV55MnVdMAVs7SZ2AiP4R 4qAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691050061; x=1691654861; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uPOhncDHb3UGNlSGuoarFvMLM7D6Id86W7w4DtvEWCw=; b=dKcWyzIihQL54tdpLinCpcfKL8zqnArIkOFgNq/H8uMALVD6Q88FsLYFq8zfSv+946 g4bGSv1JcfV81wDwQuEch/SAwjuhSgJRtYfvO//NF8Y+Kbweeh3nbwqdSEhw75u0B8TX 0k47upmoPPzCpLboUEK+UBgbyE+D/NZsg35prixBJXtir+OZMLhyMQ0stKCzpG4fCGSH x7D1X2COQ8E3srX9ZGmclcuHiH7/hVuPTaBsfVHwB9Ig1kILWKTjOXAPN6KS8jIZjKfA SPD5Zlx5ebhLV3Wwg5eN7Jign9KpiU92GpOWXg+Sp80OE1EJ/OZdpb31UEA4vf9RpkkR s5yQ== X-Gm-Message-State: ABy/qLZvCifW6a8GCdhfjWY0iC3/0ldWz2zflOWJ8Tbn2Y8tBm7ihDXh VFwHu5K4JYUkODqtojAHw+OAUQgRRgyo8U7CM7Xpng== X-Google-Smtp-Source: APBJJlGn1+ttUcGPrOJJ6OY5s9klDFtOmotn6nuslIXkO5kBdK/asFvreqe9gMizxmQoR+MQIEG+RQEWLc4dYdYBmPc= X-Received: by 2002:adf:e3ca:0:b0:316:ef23:9276 with SMTP id k10-20020adfe3ca000000b00316ef239276mr7060564wrm.52.1691050060808; Thu, 03 Aug 2023 01:07:40 -0700 (PDT) MIME-Version: 1.0 References: <20230731170756.2130927-1-apinski@marvell.com> In-Reply-To: From: Prathamesh Kulkarni Date: Thu, 3 Aug 2023 13:37:04 +0530 Message-ID: Subject: Re: [COMMITTEDv3] tree-optimization: [PR100864] `(a&!b) | b` is not opimized to `a | b` for comparisons To: Andrew Pinski Cc: Andrew Pinski , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 3 Aug 2023 at 02:54, Andrew Pinski wrote: > > On Wed, Aug 2, 2023 at 10:14=E2=80=AFAM Andrew Pinski = wrote: > > > > On Wed, Aug 2, 2023 at 10:13=E2=80=AFAM Prathamesh Kulkarni via Gcc-pat= ches > > wrote: > > > > > > On Mon, 31 Jul 2023 at 22:39, Andrew Pinski via Gcc-patches > > > wrote: > > > > > > > > This is a new version of the patch. > > > > Instead of doing the matching of inversion comparison directly insi= de > > > > match, creating a new function (bitwise_inverted_equal_p) to do it. > > > > It is very similar to bitwise_equal_p that was added in r14-2751-g2= a3556376c69a1fb > > > > but instead it says `expr1 =3D=3D ~expr2`. A follow on patch, will > > > > use this function in other patterns where we try to match `@0` and = `(bit_not @0)`. > > > > > > > > Changed the name bitwise_not_equal_p to bitwise_inverted_equal_p. > > > > > > > > Committed as approved after a Bootstrapped and test on x86_64-linux= -gnu with no regressions. > > > Hi Andrew, > > > Unfortunately, this patch (committed in > > > 2bae476b511dc441bf61da8a49cca655575e7dd6) causes > > > segmentation fault for pr33133.c on aarch64-linux-gnu because of > > > infinite recursion. > > > > A similar issue is recorded as PR 110874 which I am debugging right now= . > > Yes the issue is the same and is solved by the same patch. That's great, thanks for the heads up! Thanks, Prathamesh > > Thanks, > Andrew > > > > > Thanks, > > Andrew > > > > > > > > Running the test under gdb shows: > > > Program received signal SIGSEGV, Segmentation fault. > > > operand_compare::operand_equal_p (this=3D0x29dc680 > > > , arg0=3D0xfffff7789a68, arg1=3D0xfffff7789= f30, > > > flags=3D16) at ../../gcc/gcc/fold-const.cc:3088 > > > 3088 { > > > (gdb) bt > > > #0 operand_compare::operand_equal_p (this=3D0x29dc680 > > > , arg0=3D0xfffff7789a68, arg1=3D0xfffff7789= f30, > > > flags=3D16) at ../../gcc/gcc/fold-const.cc:3088 > > > #1 0x0000000000a90394 in operand_compare::verify_hash_value > > > (this=3Dthis@entry=3D0x29dc680 , > > > arg0=3Darg0@entry=3D0xfffff7789a68, arg1=3Darg1@entry=3D0xfffff77= 89f30, > > > flags=3Dflags@entry=3D0, ret=3Dret@entry=3D0xfffffc000157) > > > at ../../gcc/gcc/fold-const.cc:4074 > > > #2 0x0000000000a9351c in operand_compare::verify_hash_value > > > (ret=3D0xfffffc000157, flags=3D0, arg1=3D0xfffff7789f30, > > > arg0=3D0xfffff7789a68, this=3D0x29dc680 ) at > > > ../../gcc/gcc/fold-const.cc:4072 > > > #3 operand_compare::operand_equal_p (this=3Dthis@entry=3D0x29dc680 > > > , arg0=3Darg0@entry=3D0xfffff7789a68, > > > arg1=3Darg1@entry=3D0xfffff7789f30, flags=3Dflags@entry=3D0) at > > > ../../gcc/gcc/fold-const.cc:3090 > > > #4 0x0000000000a9791c in operand_equal_p > > > (arg0=3Darg0@entry=3D0xfffff7789a68, arg1=3Darg1@entry=3D0xfffff7789f= 30, > > > flags=3Dflags@entry=3D0) at ../../gcc/gcc/fold-const.cc:4105 > > > #5 0x0000000001d38dd0 in gimple_bitwise_inverted_equal_p > > > (expr1=3D0xfffff7789a68, expr2=3D0xfffff7789f30, valueize=3D > > > 0x112d698 ) at > > > ../../gcc/gcc/gimple-match-head.cc:284 > > > #6 0x0000000001d38e80 in gimple_bitwise_inverted_equal_p > > > (expr1=3D0xfffff7789a68, expr2=3D0xfffff77d0240, > > > valueize=3D0x112d698 ) at > > > ../../gcc/gcc/gimple-match-head.cc:296 > > > #7 0x0000000001d38e80 in gimple_bitwise_inverted_equal_p > > > (expr1=3D0xfffff7789a68, expr2=3D0xfffff7789f30, > > > valueize=3D0x112d698 ) at > > > ../../gcc/gcc/gimple-match-head.cc:296 > > > #8 0x0000000001d38e80 in gimple_bitwise_inverted_equal_p > > > (expr1=3D0xfffff7789a68, expr2=3D0xfffff77d0240, > > > ... > > > > > > It seems to recurse cyclically with expr2=3D0xfffff7789f30 -> > > > expr2=3D0xfffff77d0240 eventually leading to segfault. > > > while expr1=3D0xfffff7789a68 remains same throughout the stack frames= . > > > > > > Thanks, > > > Prathamesh > > > > > > > > PR tree-optimization/100864 > > > > > > > > gcc/ChangeLog: > > > > > > > > * generic-match-head.cc (bitwise_inverted_equal_p): New fun= ction. > > > > * gimple-match-head.cc (bitwise_inverted_equal_p): New macr= o. > > > > (gimple_bitwise_inverted_equal_p): New function. > > > > * match.pd ((~x | y) & x): Use bitwise_inverted_equal_p > > > > instead of direct matching bit_not. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.dg/tree-ssa/bitops-3.c: New test. > > > > --- > > > > gcc/generic-match-head.cc | 42 ++++++++++++++ > > > > gcc/gimple-match-head.cc | 71 ++++++++++++++++++++= ++++ > > > > gcc/match.pd | 5 +- > > > > gcc/testsuite/gcc.dg/tree-ssa/bitops-3.c | 67 ++++++++++++++++++++= ++ > > > > 4 files changed, 183 insertions(+), 2 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/bitops-3.c > > > > > > > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > > > > index a71c0727b0b..ddaf22f2179 100644 > > > > --- a/gcc/generic-match-head.cc > > > > +++ b/gcc/generic-match-head.cc > > > > @@ -121,3 +121,45 @@ bitwise_equal_p (tree expr1, tree expr2) > > > > return wi::to_wide (expr1) =3D=3D wi::to_wide (expr2); > > > > return operand_equal_p (expr1, expr2, 0); > > > > } > > > > + > > > > +/* Return true if EXPR1 and EXPR2 have the bitwise opposite value, > > > > + but not necessarily same type. > > > > + The types can differ through nop conversions. */ > > > > + > > > > +static inline bool > > > > +bitwise_inverted_equal_p (tree expr1, tree expr2) > > > > +{ > > > > + STRIP_NOPS (expr1); > > > > + STRIP_NOPS (expr2); > > > > + if (expr1 =3D=3D expr2) > > > > + return false; > > > > + if (!tree_nop_conversion_p (TREE_TYPE (expr1), TREE_TYPE (expr2)= )) > > > > + return false; > > > > + if (TREE_CODE (expr1) =3D=3D INTEGER_CST && TREE_CODE (expr2) = =3D=3D INTEGER_CST) > > > > + return wi::to_wide (expr1) =3D=3D ~wi::to_wide (expr2); > > > > + if (operand_equal_p (expr1, expr2, 0)) > > > > + return false; > > > > + if (TREE_CODE (expr1) =3D=3D BIT_NOT_EXPR > > > > + && bitwise_equal_p (TREE_OPERAND (expr1, 0), expr2)) > > > > + return true; > > > > + if (TREE_CODE (expr2) =3D=3D BIT_NOT_EXPR > > > > + && bitwise_equal_p (expr1, TREE_OPERAND (expr2, 0))) > > > > + return true; > > > > + if (COMPARISON_CLASS_P (expr1) > > > > + && COMPARISON_CLASS_P (expr2)) > > > > + { > > > > + tree op10 =3D TREE_OPERAND (expr1, 0); > > > > + tree op20 =3D TREE_OPERAND (expr2, 0); > > > > + if (!operand_equal_p (op10, op20)) > > > > + return false; > > > > + tree op11 =3D TREE_OPERAND (expr1, 1); > > > > + tree op21 =3D TREE_OPERAND (expr2, 1); > > > > + if (!operand_equal_p (op11, op21)) > > > > + return false; > > > > + if (invert_tree_comparison (TREE_CODE (expr1), > > > > + HONOR_NANS (op10)) > > > > + =3D=3D TREE_CODE (expr2)) > > > > + return true; > > > > + } > > > > + return false; > > > > +} > > > > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc > > > > index 5d6d26d009b..0265e55be93 100644 > > > > --- a/gcc/gimple-match-head.cc > > > > +++ b/gcc/gimple-match-head.cc > > > > @@ -263,3 +263,74 @@ gimple_bitwise_equal_p (tree expr1, tree expr2= , tree (*valueize) (tree)) > > > > return true; > > > > return false; > > > > } > > > > + > > > > +/* Return true if EXPR1 and EXPR2 have the bitwise opposite value, > > > > + but not necessarily same type. > > > > + The types can differ through nop conversions. */ > > > > +#define bitwise_inverted_equal_p(expr1, expr2) \ > > > > + gimple_bitwise_inverted_equal_p (expr1, expr2, valueize) > > > > + > > > > +/* Helper function for bitwise_equal_p macro. */ > > > > + > > > > +static inline bool > > > > +gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*va= lueize) (tree)) > > > > +{ > > > > + if (expr1 =3D=3D expr2) > > > > + return false; > > > > + if (!tree_nop_conversion_p (TREE_TYPE (expr1), TREE_TYPE (expr2)= )) > > > > + return false; > > > > + if (TREE_CODE (expr1) =3D=3D INTEGER_CST && TREE_CODE (expr2) = =3D=3D INTEGER_CST) > > > > + return wi::to_wide (expr1) =3D=3D ~wi::to_wide (expr2); > > > > + if (operand_equal_p (expr1, expr2, 0)) > > > > + return false; > > > > + > > > > + tree other; > > > > + if (gimple_nop_convert (expr1, &other, valueize) > > > > + && gimple_bitwise_inverted_equal_p (other, expr2, valueize)) > > > > + return true; > > > > + > > > > + if (gimple_nop_convert (expr2, &other, valueize) > > > > + && gimple_bitwise_inverted_equal_p (expr1, other, valueize)) > > > > + return true; > > > > + > > > > + if (TREE_CODE (expr1) !=3D SSA_NAME > > > > + || TREE_CODE (expr2) !=3D SSA_NAME) > > > > + return false; > > > > + > > > > + gimple *d1 =3D get_def (valueize, expr1); > > > > + gassign *a1 =3D safe_dyn_cast (d1); > > > > + gimple *d2 =3D get_def (valueize, expr2); > > > > + gassign *a2 =3D safe_dyn_cast (d2); > > > > + if (a1 > > > > + && gimple_assign_rhs_code (a1) =3D=3D BIT_NOT_EXPR > > > > + && gimple_bitwise_equal_p (do_valueize (valueize, > > > > + gimple_assign_rhs1 (a= 1)), > > > > + expr2, valueize)) > > > > + return true; > > > > + if (a2 > > > > + && gimple_assign_rhs_code (a2) =3D=3D BIT_NOT_EXPR > > > > + && gimple_bitwise_equal_p (expr1, > > > > + do_valueize (valueize, > > > > + gimple_assign_rhs1 (a= 2)), > > > > + valueize)) > > > > + return true; > > > > + > > > > + if (a1 && a2 > > > > + && TREE_CODE_CLASS (gimple_assign_rhs_code (a1)) =3D=3D tcc_= comparison > > > > + && TREE_CODE_CLASS (gimple_assign_rhs_code (a2)) =3D=3D tcc_= comparison) > > > > + { > > > > + tree op10 =3D gimple_assign_rhs1 (a1); > > > > + tree op20 =3D gimple_assign_rhs1 (a2); > > > > + if (!operand_equal_p (op10, op20)) > > > > + return false; > > > > + tree op11 =3D gimple_assign_rhs2 (a1); > > > > + tree op21 =3D gimple_assign_rhs2 (a2); > > > > + if (!operand_equal_p (op11, op21)) > > > > + return false; > > > > + if (invert_tree_comparison (gimple_assign_rhs_code (a1), > > > > + HONOR_NANS (op10)) > > > > + =3D=3D gimple_assign_rhs_code (a2)) > > > > + return true; > > > > + } > > > > + return false; > > > > +} > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > > > index ee6cef6b09d..5fc6f517ab9 100644 > > > > --- a/gcc/match.pd > > > > +++ b/gcc/match.pd > > > > @@ -1943,8 +1943,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > > > /* (~x | y) & x -> x & y */ > > > > /* (~x & y) | x -> x | y */ > > > > (simplify > > > > - (bitop:c (rbitop:c (bit_not @0) @1) @0) > > > > - (bitop @0 @1))) > > > > + (bitop:c (rbitop:c @2 @1) @0) > > > > + (if (bitwise_inverted_equal_p (@0, @2)) > > > > + (bitop @0 @1)))) > > > > > > > > /* ((x | y) & z) | x -> (z & y) | x */ > > > > (simplify > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/bitops-3.c b/gcc/testsui= te/gcc.dg/tree-ssa/bitops-3.c > > > > new file mode 100644 > > > > index 00000000000..bf11a129b69 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/bitops-3.c > > > > @@ -0,0 +1,67 @@ > > > > +/* PR tree-optimization/100864 */ > > > > + > > > > +/* { dg-do run } */ > > > > +/* { dg-options "-O1 -fdump-tree-optimized-raw" } */ > > > > + > > > > +#define op_ne !=3D > > > > +#define op_eq =3D=3D > > > > +#define op_lt < > > > > +#define op_le <=3D > > > > +#define op_gt > > > > > +#define op_ge >=3D > > > > + > > > > +#define operators(t) \ > > > > +t(ne) \ > > > > +t(eq) \ > > > > +t(lt) \ > > > > +t(le) \ > > > > +t(gt) \ > > > > +t(ge) > > > > + > > > > +#define cmpfunc(v, op) \ > > > > +__attribute__((noipa)) \ > > > > +_Bool func_##op##_##v(v int a, v int b, v _Bool e) \ > > > > +{ \ > > > > + v _Bool c =3D (a op_##op b); \ > > > > + v _Bool d =3D !c; \ > > > > + return (e & d) | c; \ > > > > +} > > > > + > > > > +#define cmp_funcs(op) \ > > > > +cmpfunc(, op) \ > > > > +cmpfunc(volatile , op) > > > > + > > > > +operators(cmp_funcs) > > > > + > > > > +#define test(op) \ > > > > +if (func_##op##_ (a, b, e) !=3D func_##op##_volatile (a, b, e)) \ > > > > + __builtin_abort(); > > > > + > > > > +int main() > > > > +{ > > > > + for(int a =3D -3; a <=3D 3; a++) > > > > + for(int b =3D -3; b <=3D 3; b++) > > > > + { > > > > + _Bool e =3D 0; > > > > + operators(test) > > > > + e =3D 1; > > > > + operators(test) > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +/* Check to make sure we optimize `(a&!b) | b` -> `a | b`. */ > > > > +/* There are 6 different comparison operators testing here. */ > > > > +/* bit_not_expr and bit_and_expr should show up for each one (vola= tile). */ > > > > +/* Each operator should show up twice > > > > + (except for `!=3D` which shows up 2*6 (each tester) + 2 (the 2 = loops) extra =3D 16). */ > > > > +/* bit_ior_expr will show up for each operator twice (non-volatile= and volatile). */ > > > > +/* { dg-final { scan-tree-dump-times "ne_expr," 16 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "eq_expr," 2 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "lt_expr," 2 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "le_expr," 2 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "gt_expr," 2 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "ge_expr," 2 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "bit_not_expr," 6 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "bit_and_expr," 6 "optimized= "} } */ > > > > +/* { dg-final { scan-tree-dump-times "bit_ior_expr," 12 "optimized= "} } */ > > > > -- > > > > 2.31.1 > > > >