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 <ja...@redhat.com> > Andrew Pinski <apin...@marvell.com> > > 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 <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)