From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 709B93858D37 for ; Thu, 3 Feb 2022 08:34:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 709B93858D37 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 8468F21110; Thu, 3 Feb 2022 08:34:55 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 7E965A3B90; Thu, 3 Feb 2022 08:34:55 +0000 (UTC) Date: Thu, 3 Feb 2022 09:34:55 +0100 (CET) From: Richard Biener To: Jakub Jelinek cc: Andrew MacLeod , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] ranger: Fix up wi_fold_in_parts for small precision types [PR104334] In-Reply-To: <20220203075634.GF2646553@tucnak> Message-ID: <1948p5o7-n422-32q2-56nq-1qrp88364sn2@fhfr.qr> References: <20220203075634.GF2646553@tucnak> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SOMETLD_ARE_BAD_TLD, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no 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: Thu, 03 Feb 2022 08:34:57 -0000 On Thu, 3 Feb 2022, Jakub Jelinek wrote: > Hi! > > The wide-int.h templates expect that when an int/long etc. operand is used > it will be sign-extended based on the types precision. > wi_fold_in_parts passes 3 such non-zero constants to wi::lt_p, wi::gt_p > and wi::eq_p - 1, 3 and 4, which means it was doing weird things if either > some of 1, 3 or 4 weren't representable in type, or if type was unsigned 3 bit > type 4 should be written as -4. > The following patch promotes the subtraction operands to widest_int and > uses that as the type for ?h_range variables and compares them as such. > We don't need the overflow handling because there is never an overflow. > > Bootstrapped/regtested on x86_64-linux and i686-linux (on the former also > with lto bootstrap), ok for trunk? OK. Thanks, Richard. > 2022-02-02 Jakub Jelinek > > PR tree-optimization/104334 > * range-op.cc (range_operator::wi_fold_in_parts): Change lh_range > and rh_range type to widest_int and subtract in widest_int. Remove > ov_rh, ov_lh and sign vars, always perform comparisons as signed > and use >, < and == operators for it. > > * g++.dg/opt/pr104334.C: New test. > > --- gcc/range-op.cc.jj 2022-01-13 22:29:15.345831749 +0100 > +++ gcc/range-op.cc 2022-02-02 20:20:22.020000000 +0100 > @@ -144,22 +144,21 @@ range_operator::wi_fold_in_parts (irange > const wide_int &rh_lb, > const wide_int &rh_ub) const > { > - wi::overflow_type ov_rh, ov_lh; > int_range_max tmp; > - wide_int rh_range = wi::sub (rh_ub, rh_lb, TYPE_SIGN (type), &ov_rh); > - wide_int lh_range = wi::sub (lh_ub, lh_lb, TYPE_SIGN (type), &ov_lh); > - signop sign = TYPE_SIGN (type);; > + widest_int rh_range = wi::sub (widest_int::from (rh_ub, TYPE_SIGN (type)), > + widest_int::from (rh_lb, TYPE_SIGN (type))); > + widest_int lh_range = wi::sub (widest_int::from (lh_ub, TYPE_SIGN (type)), > + widest_int::from (lh_lb, TYPE_SIGN (type))); > // If there are 2, 3, or 4 values in the RH range, do them separately. > // Call wi_fold_in_parts to check the RH side. > - if (wi::gt_p (rh_range, 0, sign) && wi::lt_p (rh_range, 4, sign) > - && ov_rh == wi::OVF_NONE) > + if (rh_range > 0 && rh_range < 4) > { > wi_fold_in_parts (r, type, lh_lb, lh_ub, rh_lb, rh_lb); > - if (wi::gt_p (rh_range, 1, sign)) > + if (rh_range > 1) > { > wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 1, rh_lb + 1); > r.union_ (tmp); > - if (wi::eq_p (rh_range, 3)) > + if (rh_range == 3) > { > wi_fold_in_parts (tmp, type, lh_lb, lh_ub, rh_lb + 2, rh_lb + 2); > r.union_ (tmp); > @@ -170,15 +169,14 @@ range_operator::wi_fold_in_parts (irange > } > // Otherise check for 2, 3, or 4 values in the LH range and split them up. > // The RH side has been checked, so no recursion needed. > - else if (wi::gt_p (lh_range, 0, sign) && wi::lt_p (lh_range, 4, sign) > - && ov_lh == wi::OVF_NONE) > + else if (lh_range > 0 && lh_range < 4) > { > wi_fold (r, type, lh_lb, lh_lb, rh_lb, rh_ub); > - if (wi::gt_p (lh_range, 1, sign)) > + if (lh_range > 1) > { > wi_fold (tmp, type, lh_lb + 1, lh_lb + 1, rh_lb, rh_ub); > r.union_ (tmp); > - if (wi::eq_p (lh_range, 3)) > + if (lh_range == 3) > { > wi_fold (tmp, type, lh_lb + 2, lh_lb + 2, rh_lb, rh_ub); > r.union_ (tmp); > --- gcc/testsuite/g++.dg/opt/pr104334.C.jj 2022-02-02 14:35:51.184657968 +0100 > +++ gcc/testsuite/g++.dg/opt/pr104334.C 2022-02-02 14:37:14.888478594 +0100 > @@ -0,0 +1,40 @@ > +// PR tree-optimization/104334 > +// { dg-do run { target c++11 } } > +// { dg-options "-O2 --param logical-op-non-short-circuit=0" } > + > +enum class A { A0, A1, A2, A3 }; > +int x; > + > +__attribute__((noipa)) void > +baz () > +{ > + x = 1; > +} > + > +struct B { > + unsigned b : 2; > + > + A > + foo () const > + { > + return static_cast (b); > + } > + > + __attribute__((noinline)) void > + bar () > + { > + if (foo () == A::A2 || foo () == A::A3) > + baz (); > + } > +}; > + > +int > +main () > +{ > + B c; > + c.b = 2; > + c.bar (); > + if (x != 1) > + __builtin_abort (); > + return 0; > +} > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)