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)

Reply via email to