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)
