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 52A383858D28 for ; Mon, 31 Jan 2022 08:28:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 52A383858D28 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 4389C210F6; Mon, 31 Jan 2022 08:28:56 +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 3D854A3B84; Mon, 31 Jan 2022 08:28:56 +0000 (UTC) Date: Mon, 31 Jan 2022 09:28:56 +0100 (CET) From: Richard Biener To: Jakub Jelinek cc: Jeff Law , Zhao Wei Liew , GCC Patches Subject: Re: [PATCH] match.pd: Fix up 1 / X for unsigned X optimization [PR104280] In-Reply-To: <20220129162334.GV2646553@tucnak> Message-ID: <7p9p3n3q-sp3p-s4po-oo9p-s8s2n1o788@fhfr.qr> References: <20220129162334.GV2646553@tucnak> MIME-Version: 1.0 X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT 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: Mon, 31 Jan 2022 08:28:58 -0000 On Sat, 29 Jan 2022, Jakub Jelinek wrote: > Hi! > > On Fri, Jan 28, 2022 at 11:38:23AM -0700, Jeff Law wrote: > > Thanks.  Given the original submission and most of the review work was done > > prior to stage3 closing, I went ahead and installed this on the trunk. > > Unfortunately this breaks quite a lot of things. > The main problem is that GIMPLE allows EQ_EXPR etc. only with BOOLEAN_TYPE > or with TYPE_PRECISION == 1 integral type (or vector boolean). > Violating this causes verification failures in tree-cfg.cc in some cases, > in other cases wrong-code issues because before it is verified we e.g. > transform > 1U / x > into > x == 1U > and later into > x (because we assume that == type must be one of the above cases and > when it is the same type as the type of the first operand, for boolean-ish > cases it should be equivalent). > > Fixed by changing that > (eq @1 { build_one_cst (type); }) > into > (convert (eq:boolean_type_node @1 { build_one_cst (type); })) > Note, I'm not 100% sure if :boolean_type_node is required in that case, It shouldn't be necessary. Unfortunately it's hard for genmatch to statically detect this case as bogus ... Thanks for spotting and fixing this. Richard. > I see some spots in match.pd that look exactly like this, while there is > e.g. (convert (le ...)) that supposedly does the right thing too. > The signed integer 1/X case doesn't need changes changes, for > (cond (le ...) ...) > le gets correctly boolean_type_node and cond should use type. > I've also reformatted it, some lines were too long, match.pd uses > indentation by 1 column instead of 2 etc. > > Bootstrapped/regtested on powerpc64le-linux and tested on the testcases > on x86_64-linux, ok for trunk? > > 2022-01-29 Jakub Jelinek > Andrew Pinski > > PR tree-optimization/104279 > PR tree-optimization/104280 > PR tree-optimization/104281 > * match.pd (1 / X -> X == 1 for unsigned X): Build eq with > boolean_type_node and convert to type. Formatting fixes. > > * gcc.dg/torture/pr104279.c: New test. > * gcc.dg/torture/pr104280.c: New test. > * gcc.dg/torture/pr104281.c: New test. > > --- gcc/match.pd.jj 2022-01-29 11:11:39.316628007 +0100 > +++ gcc/match.pd 2022-01-29 12:19:46.662096678 +0100 > @@ -435,18 +435,22 @@ (define_operator_list SYNC_FETCH_AND_AND > && TYPE_UNSIGNED (type)) > (trunc_divmod @0 @1)))) > > - /* 1 / X -> X == 1 for unsigned integer X. > - 1 / X -> X >= -1 && X <= 1 ? X : 0 for signed integer X. > - But not for 1 / 0 so that we can get proper warnings and errors, > - and not for 1-bit integers as they are edge cases better handled elsewhere. */ > +/* 1 / X -> X == 1 for unsigned integer X. > + 1 / X -> X >= -1 && X <= 1 ? X : 0 for signed integer X. > + But not for 1 / 0 so that we can get proper warnings and errors, > + and not for 1-bit integers as they are edge cases better handled > + elsewhere. */ > (simplify > - (trunc_div integer_onep@0 @1) > - (if (INTEGRAL_TYPE_P (type) && !integer_zerop (@1) && TYPE_PRECISION (type) > 1) > - (if (TYPE_UNSIGNED (type)) > - (eq @1 { build_one_cst (type); }) > - (with { tree utype = unsigned_type_for (type); } > - (cond (le (plus (convert:utype @1) { build_one_cst (utype); }) { build_int_cst (utype, 2); }) > - @1 { build_zero_cst (type); }))))) > + (trunc_div integer_onep@0 @1) > + (if (INTEGRAL_TYPE_P (type) > + && !integer_zerop (@1) > + && TYPE_PRECISION (type) > 1) > + (if (TYPE_UNSIGNED (type)) > + (convert (eq:boolean_type_node @1 { build_one_cst (type); })) > + (with { tree utype = unsigned_type_for (type); } > + (cond (le (plus (convert:utype @1) { build_one_cst (utype); }) > + { build_int_cst (utype, 2); }) > + @1 { build_zero_cst (type); }))))) > > /* Combine two successive divisions. Note that combining ceil_div > and floor_div is trickier and combining round_div even more so. */ > --- gcc/testsuite/gcc.dg/torture/pr104279.c.jj 2022-01-29 12:25:36.388174312 +0100 > +++ gcc/testsuite/gcc.dg/torture/pr104279.c 2022-01-29 12:25:53.206937588 +0100 > @@ -0,0 +1,12 @@ > +/* PR tree-optimization/104279 */ > +/* { dg-do compile } */ > + > +unsigned a, b; > + > +int > +main () > +{ > + b = ~(0 || ~0); > + a = ~b / ~a; > + return 0; > +} > --- gcc/testsuite/gcc.dg/torture/pr104280.c.jj 2022-01-29 12:24:36.190021595 +0100 > +++ gcc/testsuite/gcc.dg/torture/pr104280.c 2022-01-29 12:25:28.681282783 +0100 > @@ -0,0 +1,16 @@ > +/* PR tree-optimization/104280 */ > +/* { dg-do run } */ > + > +int > +foo (unsigned b, int c) > +{ > + return b / c; > +} > + > +int > +main () > +{ > + if (foo (1, 2) != 0) > + __builtin_abort (); > + return 0; > +} > --- gcc/testsuite/gcc.dg/torture/pr104281.c.jj 2022-01-29 12:27:29.840577473 +0100 > +++ gcc/testsuite/gcc.dg/torture/pr104281.c 2022-01-29 12:27:24.526652267 +0100 > @@ -0,0 +1,22 @@ > +/* PR tree-optimization/104281 */ > +/* { dg-do run } */ > + > +unsigned a = 1; > +int b, c = 2; > +long d; > + > +int > +main () > +{ > + while (1) > + { > + int m = a; > + L: > + a = ~(-(m || b & d)); > + b = ((1 ^ a) / c); > + if (b) > + goto L; > + break; > + } > + return 0; > +} > > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)