From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by sourceware.org (Postfix) with ESMTPS id 540B93858D20 for ; Fri, 11 Aug 2023 06:46:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 540B93858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x233.google.com with SMTP id 38308e7fff4ca-2ba0f27a4c2so25139571fa.2 for ; Thu, 10 Aug 2023 23:46:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1691736392; x=1692341192; 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=LL78nDCz7mUuOn535EkPh+LS6mAEhwphsd9SMKt+tjA=; b=CHvrlvw2NU/5ekGZpOCwkSP/0aFSidgaTiMeeb8PybcRaPDD9IbTP2bYZOyxTCjW+i 3OtWdqBdnX3YYM+jm5aMBm339SETUb7gK89z8dkJQWBzqliJ4YDsOjpgPABsMzbOMuSz 4HhHB0o9wJ0zj9hQklkwyS9AAm7etny7Y9003Sy67eLHsXrt1YXGiTCBOQS5pIxZT/uk EhCcXBgh/H7Rh89kcN2zUhmd4gLPF/VMnHGnCn10oujOjylUodalMKph10+iMs8KYtgp 20AT1LHt3VJdlE0IuXqPZAoFQ43KyYPQ/C3Rfn/4XO+tlmblOdIsNCqdwRoai2Yy5qlC nIsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691736392; x=1692341192; 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=LL78nDCz7mUuOn535EkPh+LS6mAEhwphsd9SMKt+tjA=; b=giiY9KXdYvS4aoM1sytCWv84opt+drPH01lDiS6Lj6NILWW/KdqMISRKuAnLarAtAG iPbMQ79Ebf33bbsUH2XdsQp2rsG1CpDuSSHDT+gj6hPb3ljS4Xw4aejZJSmocCgrX2rD vJxY3TkUcfImLq1Id9KVQ1/oGXwcSFpOTfFQp8cgQmEEmI7jS4Gbf8qZq67c2JAEsARv g5dRcHfegXrHaXT9+NjwS1Zmu0unru1K6gg8w44uPo2kt7/svxIaLjqaK18mclrz/E59 95hM5yQtr58zWRuTuB17pr5HZsf59BU4MyHbEhykRXflrL+goCtXw50Q3h7KqJMywudK b6Vg== X-Gm-Message-State: AOJu0YxnPTl2hQ9zKxmZIuwtWADeYDLobXWqz3Yvhyic4ZDuOduyDa6N XPMxj6eypzRAX4qiSeK1fkJW2dzTw22bhF0uxMjsJCmI X-Google-Smtp-Source: AGHT+IHAGVi7gjY/OOc8k7/afDnyRnvBLQMTNZH7rd10LZHGCD8H6x3z1OBhtMEnKlUAXbRSZO4E8nx8wi1mNZ3M9do= X-Received: by 2002:a2e:8297:0:b0:2b6:dd9a:e1d3 with SMTP id y23-20020a2e8297000000b002b6dd9ae1d3mr896863ljg.44.1691736391573; Thu, 10 Aug 2023 23:46:31 -0700 (PDT) MIME-Version: 1.0 References: <20230810225121.2732876-1-apinski@marvell.com> In-Reply-To: <20230810225121.2732876-1-apinski@marvell.com> From: Richard Biener Date: Fri, 11 Aug 2023 08:46:18 +0200 Message-ID: Subject: Re: [PATCHv2] Fix PR 110954: wrong code with cmp | !cmp To: Andrew Pinski Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Aug 11, 2023 at 12:52=E2=80=AFAM Andrew Pinski via Gcc-patches wrote: > > This was an oversight on my part forgetting that > cmp will might have a different true value than all ones > but will have a value of 1 in most cases. > This means if we have `(f < 0) | !(f < 0)` we would > optimize this to -1 rather than just 1. > > This is version 2 of the patch. > Decided to go down a different route than just checking if > the precission was 1 inside bitwise_inverted_equal_p. > So instead bitwise_inverted_equal_p gets passed an argument > that will be set if there was a comparison that was being compared > and the user of bitwise_inverted_equal_p decides what needs to be done. > In most uses of bitwise_inverted_equal_p, the check will be > `!wascmp || element_precision (type) =3D=3D 1` . > But in the case of `a & ~a` and `a ^| ~a` we can handle the case > of wascmp by using constant_boolean_node isntead. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK. Thanks, Richard. > PR 110954 > > gcc/ChangeLog: > > * generic-match-head.cc (bitwise_inverted_equal_p): Add > wascmp argument and set it accordingly. > * gimple-match-head.cc (bitwise_inverted_equal_p): Add > wascmp argument to the macro. > (gimple_bitwise_inverted_equal_p): Add > wascmp argument and set it accordingly. > * match.pd (`a & ~a`, `a ^| ~a`): Update call > to bitwise_inverted_equal_p and handle wascmp case. > (`(~x | y) & x`, `(~x | y) & x`, `a?~t:t`): Update > call to bitwise_inverted_equal_p and check to see > if was !wascmp or if precision was 1. > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/execute/pr110954-1.c: New test. > --- > gcc/generic-match-head.cc | 4 +- > gcc/gimple-match-head.cc | 8 ++-- > gcc/match.pd | 41 +++++++++++-------- > .../gcc.c-torture/execute/pr110954-1.c | 10 +++++ > 4 files changed, 43 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110954-1.c > > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc > index ddaf22f2179..f40a35c48a6 100644 > --- a/gcc/generic-match-head.cc > +++ b/gcc/generic-match-head.cc > @@ -127,10 +127,11 @@ bitwise_equal_p (tree expr1, tree expr2) > The types can differ through nop conversions. */ > > static inline bool > -bitwise_inverted_equal_p (tree expr1, tree expr2) > +bitwise_inverted_equal_p (tree expr1, tree expr2, bool &wascmp) > { > STRIP_NOPS (expr1); > STRIP_NOPS (expr2); > + wascmp =3D false; > if (expr1 =3D=3D expr2) > return false; > if (!tree_nop_conversion_p (TREE_TYPE (expr1), TREE_TYPE (expr2))) > @@ -150,6 +151,7 @@ bitwise_inverted_equal_p (tree expr1, tree expr2) > { > tree op10 =3D TREE_OPERAND (expr1, 0); > tree op20 =3D TREE_OPERAND (expr2, 0); > + wascmp =3D true; > if (!operand_equal_p (op10, op20)) > return false; > tree op11 =3D TREE_OPERAND (expr1, 1); > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc > index a097a494c39..ea6387a1099 100644 > --- a/gcc/gimple-match-head.cc > +++ b/gcc/gimple-match-head.cc > @@ -267,8 +267,8 @@ gimple_bitwise_equal_p (tree expr1, tree expr2, tree = (*valueize) (tree)) > /* 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) > +#define bitwise_inverted_equal_p(expr1, expr2, wascmp) \ > + gimple_bitwise_inverted_equal_p (expr1, expr2, wascmp, valueize) > > > bool gimple_bit_not_with_nop (tree, tree *, tree (*) (tree)); > @@ -277,8 +277,9 @@ bool gimple_maybe_cmp (tree, tree *, tree (*) (tree))= ; > /* Helper function for bitwise_equal_p macro. */ > > static inline bool > -gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize= ) (tree)) > +gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, bool &wascmp, t= ree (*valueize) (tree)) > { > + wascmp =3D false; > if (expr1 =3D=3D expr2) > return false; > if (!tree_nop_conversion_p (TREE_TYPE (expr1), TREE_TYPE (expr2))) > @@ -331,6 +332,7 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree exp= r2, tree (*valueize) (tree) > tree op21 =3D do_valueize (valueize, gimple_assign_rhs2 (a2)); > if (!operand_equal_p (op11, op21)) > return false; > + wascmp =3D true; > if (invert_tree_comparison (gimple_assign_rhs_code (a1), > HONOR_NANS (op10)) > =3D=3D gimple_assign_rhs_code (a2)) > diff --git a/gcc/match.pd b/gcc/match.pd > index fc630b63563..6791060891d 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -1179,9 +1179,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* Simplify ~X & X as zero. */ > (simplify > (bit_and (convert? @0) (convert? @1)) > - (if (types_match (TREE_TYPE (@0), TREE_TYPE (@1)) > - && bitwise_inverted_equal_p (@0, @1)) > - { build_zero_cst (type); })) > + (with { bool wascmp; } > + (if (types_match (TREE_TYPE (@0), TREE_TYPE (@1)) > + && bitwise_inverted_equal_p (@0, @1, wascmp)) > + { wascmp ? constant_boolean_node (false, type) : build_zero_cst (type= ); }))) > > /* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b); */ > (simplify > @@ -1419,9 +1420,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > (for op (bit_ior bit_xor) > (simplify > (op (convert? @0) (convert? @1)) > - (if (types_match (TREE_TYPE (@0), TREE_TYPE (@1)) > - && bitwise_inverted_equal_p (@0, @1)) > - (convert { build_all_ones_cst (TREE_TYPE (@0)); })))) > + (with { bool wascmp; } > + (if (types_match (TREE_TYPE (@0), TREE_TYPE (@1)) > + && bitwise_inverted_equal_p (@0, @1, wascmp)) > + (convert > + { wascmp > + ? constant_boolean_node (true, type) > + : build_all_ones_cst (TREE_TYPE (@0)); }))))) > > /* x ^ x -> 0 */ > (simplify > @@ -1969,8 +1974,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* (~x & y) | x -> x | y */ > (simplify > (bitop:c (rbitop:c @2 @1) @0) > - (if (bitwise_inverted_equal_p (@0, @2)) > - (bitop @0 @1)))) > + (with { bool wascmp; } > + (if (bitwise_inverted_equal_p (@0, @2, wascmp) > + && (!wascmp || element_precision (type) =3D=3D 1)) > + (bitop @0 @1))))) > > /* ((x | y) & z) | x -> (z & y) | x */ > (simplify > @@ -6464,14 +6471,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* a?~t:t -> (-(a))^t */ > (simplify > (cond @0 @1 @2) > - (if (INTEGRAL_TYPE_P (type) > - && bitwise_inverted_equal_p (@1, @2)) > - (with { > - auto prec =3D TYPE_PRECISION (type); > - auto unsign =3D TYPE_UNSIGNED (type); > - tree inttype =3D build_nonstandard_integer_type (prec, unsign); > - } > - (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2))= )))) > + (with { bool wascmp; } > + (if (INTEGRAL_TYPE_P (type) > + && bitwise_inverted_equal_p (@1, @2, wascmp) > + && (!wascmp || element_precision (type) =3D=3D 1)) > + (with { > + auto prec =3D TYPE_PRECISION (type); > + auto unsign =3D TYPE_UNSIGNED (type); > + tree inttype =3D build_nonstandard_integer_type (prec, unsign); > + } > + (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)= )))))) > #endif > > /* Simplify pointer equality compares using PTA. */ > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c b/gcc/tests= uite/gcc.c-torture/execute/pr110954-1.c > new file mode 100644 > index 00000000000..8aad758e10f > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c > @@ -0,0 +1,10 @@ > + > +#define comparison (f < 0) > +int main() { > + int f =3D 0; > + int d =3D comparison | !comparison; > + if (d !=3D 1) > + __builtin_abort(); > + return 0; > +} > + > -- > 2.31.1 >