[PATCH] match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

[PATCH] match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

[Jakub Jelinek]

Hi!

I really can't figure out why one would need to add extra casts.
type must be an integral type which has BIT_NOT_EXPR applied on it
which yields all ones and we need a type in which negating 0 or 1
range will yield 0 or all ones, I think all integral types satisfy
that.
This fixes PR111369, where one of the bitint*.c tests FAILs with
GCC_TEST_RUN_EXPENSIVE=1.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-09-30  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/111369
	* match.pd (a?~t:t -> (-(a))^t): Always convert to type rather
	than using build_nonstandard_integer_type.

--- gcc/match.pd.jj	2023-09-28 11:32:16.122434235 +0200
+++ gcc/match.pd	2023-09-29 18:05:50.554640268 +0200
@@ -6742,12 +6742,7 @@ (define_operator_list SYNC_FETCH_AND_AND
   (if (INTEGRAL_TYPE_P (type)
        && bitwise_inverted_equal_p (@1, @2, wascmp)
        && (!wascmp || element_precision (type) == 1))
-   (with {
-     auto prec = TYPE_PRECISION (type);
-     auto unsign = TYPE_UNSIGNED (type);
-     tree inttype = build_nonstandard_integer_type (prec, unsign);
-    }
-    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
+   (bit_xor (negate (convert:type @0)) (convert:type @2)))))
 #endif
 
 /* Simplify pointer equality compares using PTA.  */

	Jakub

Re: [PATCH] match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

[Jakub Jelinek]

On Sat, Sep 30, 2023 at 11:44:59AM +0200, Jakub Jelinek wrote:
> I really can't figure out why one would need to add extra casts.
> type must be an integral type which has BIT_NOT_EXPR applied on it
> which yields all ones and we need a type in which negating 0 or 1
> range will yield 0 or all ones, I think all integral types satisfy
> that.
> This fixes PR111369, where one of the bitint*.c tests FAILs with
> GCC_TEST_RUN_EXPENSIVE=1.

Though, I think there is an preexisting issue which the
build_nonstandard_integer_type didn't help with; if type is signed 1-bit
precision, then I think a ? ~t : t could be valid, but -(type)a would invoke
UB if a is 1 - the cast would make it -1 and negation of -1 in signed 1-bit
invokes UB.
So perhaps we should guard this optimization on type having element precision > 1
or being unsigned.  Plus the (convert:type @2) didn't make sense, @2 already
must have TREE_TYPE type.

So untested patch would be then:

2023-09-29  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/111369
	* match.pd (a?~t:t -> (-(a))^t): Always convert to type rather
	than using build_nonstandard_integer_type.  Punt if type is
	signed 1-bit precision type.

--- gcc/match.pd.jj	2023-09-29 18:58:42.724956659 +0200
+++ gcc/match.pd	2023-09-30 11:54:16.603280666 +0200
@@ -6741,13 +6741,9 @@ (define_operator_list SYNC_FETCH_AND_AND
  (with { bool wascmp; }
   (if (INTEGRAL_TYPE_P (type)
        && bitwise_inverted_equal_p (@1, @2, wascmp)
-       && (!wascmp || element_precision (type) == 1))
-   (with {
-     auto prec = TYPE_PRECISION (type);
-     auto unsign = TYPE_UNSIGNED (type);
-     tree inttype = build_nonstandard_integer_type (prec, unsign);
-    }
-    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
+       && (!wascmp || element_precision (type) == 1)
+       && (!TYPE_OVERFLOW_WRAPS (type) || element_precision (type) > 1))
+   (bit_xor (negate (convert:type @0)) @2))))
 #endif
 
 /* Simplify pointer equality compares using PTA.  */


	Jakub

[PATCH] match.pd: Avoid other build_nonstandard_integer_type calls [PR111369]

[Jakub Jelinek]

Hi!

On Sat, Sep 30, 2023 at 11:57:38AM +0200, Jakub Jelinek wrote:
> > This fixes PR111369, where one of the bitint*.c tests FAILs with
> > GCC_TEST_RUN_EXPENSIVE=1.
> 
> Though, I think there is an preexisting issue which the
> build_nonstandard_integer_type didn't help with; if type is signed 1-bit
> precision, then I think a ? ~t : t could be valid, but -(type)a would invoke
> UB if a is 1 - the cast would make it -1 and negation of -1 in signed 1-bit
> invokes UB.
> So perhaps we should guard this optimization on type having element precision > 1
> or being unsigned.  Plus the (convert:type @2) didn't make sense, @2 already
> must have TREE_TYPE type.

In the light of the PR111668 patch which shows that
build_nonstandard_integer_type is needed (at least for some signed prec > 1
BOOLEAN_TYPEs if we use e.g. negation), I've reworked this patch and handled
the last problematic build_nonstandard_integer_type call in there as well.

In the x == cstN ? cst4 : cst3 optimization it uses
build_nonstandard_integer_type solely for BOOLEAN_TYPEs (I really don't see
why ENUMERAL_TYPEs would be a problem, we treat them in GIMPLE as uselessly
convertible to same precision/sign INTEGER_TYPEs), for INTEGER_TYPEs it is
really a no-op (might return a different type, but always INTEGER_TYPE
with same TYPE_PRECISION same TYPE_UNSIGNED) and for BITINT_TYPE with larger
precisions really harmful (we shouldn't create large precision
INTEGER_TYPEs).

The a?~t:t optimization just omits the negation of a in type for 1-bit
precision types or any BOOLEAN_TYPEs.  I think that is correct, because
for both signed and unsigned 1-bit precision type, cast to type of a bool
value yields already 0, -1 or 0, 1 values and for 1-bit precision negation
of that is still 0, -1 or 0, 1 (except for invoking sometimes UB).
And for signed larger precision BOOLEAN_TYPEs I think it is correct as well,
cast of [0, 1] to type yields 0, -1 and those can be xored with 0 or -1
to yield the proper result, any other values would be UB.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-10-03  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/111369
	* match.pd (x == cstN ? cst4 : cst3): Use
	build_nonstandard_integer_type only if type1 is BOOLEAN_TYPE.
	Fix comment typo.  Formatting fix.
	(a?~t:t -> (-(a))^t): Always convert to type rather
	than using build_nonstandard_integer_type.  Perform negation
	only if type has precision > 1 and is not signed BOOLEAN_TYPE.

--- gcc/match.pd.jj	2023-10-03 10:33:30.817614648 +0200
+++ gcc/match.pd	2023-10-03 11:29:54.089566764 +0200
@@ -5178,7 +5178,7 @@ (define_operator_list SYNC_FETCH_AND_AND
 
 /* Optimize
    # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
-   x_5 ? cstN ? cst4 : cst3
+   x_5 == cstN ? cst4 : cst3
    # op is == or != and N is 1 or 2
    to r_6 = x_5 + (min (cst3, cst4) - cst1) or
    r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
@@ -5214,7 +5214,8 @@ (define_operator_list SYNC_FETCH_AND_AND
 	 type1 = type;
        auto prec = TYPE_PRECISION (type1);
        auto unsign = TYPE_UNSIGNED (type1);
-       type1 = build_nonstandard_integer_type (prec, unsign);
+       if (TREE_CODE (type1) == BOOLEAN_TYPE)
+	type1 = build_nonstandard_integer_type (prec, unsign);
        min = wide_int::from (min, prec,
 			     TYPE_SIGN (TREE_TYPE (@0)));
        wide_int a = wide_int::from (wi::to_wide (arg0), prec,
@@ -5253,14 +5254,7 @@ (define_operator_list SYNC_FETCH_AND_AND
       }
       (if (code == PLUS_EXPR)
        (convert (plus (convert:type1 @0) { arg; }))
-       (convert (minus { arg; } (convert:type1 @0)))
-      )
-     )
-    )
-   )
-  )
- )
-)
+       (convert (minus { arg; } (convert:type1 @0))))))))))
 #endif
 
 (simplify
@@ -6758,13 +6752,11 @@ (define_operator_list SYNC_FETCH_AND_AND
  (with { bool wascmp; }
   (if (INTEGRAL_TYPE_P (type)
        && bitwise_inverted_equal_p (@1, @2, wascmp)
-       && (!wascmp || element_precision (type) == 1))
-   (with {
-     auto prec = TYPE_PRECISION (type);
-     auto unsign = TYPE_UNSIGNED (type);
-     tree inttype = build_nonstandard_integer_type (prec, unsign);
-    }
-    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
+       && (!wascmp || TYPE_PRECISION (type) == 1))
+   (if ((!TYPE_UNSIGNED (type) && TREE_CODE (type) == BOOLEAN_TYPE)
+	|| TYPE_PRECISION (type) == 1)
+    (bit_xor (convert:type @0) @2)
+    (bit_xor (negate (convert:type @0)) @2)))))
 #endif
 
 /* Simplify pointer equality compares using PTA.  */


	Jakub

Re: [PATCH] match.pd: Avoid other build_nonstandard_integer_type calls [PR111369]

[Richard Biener]

On Tue, 3 Oct 2023, Jakub Jelinek wrote:

> Hi!
> 
> On Sat, Sep 30, 2023 at 11:57:38AM +0200, Jakub Jelinek wrote:
> > > This fixes PR111369, where one of the bitint*.c tests FAILs with
> > > GCC_TEST_RUN_EXPENSIVE=1.
> > 
> > Though, I think there is an preexisting issue which the
> > build_nonstandard_integer_type didn't help with; if type is signed 1-bit
> > precision, then I think a ? ~t : t could be valid, but -(type)a would invoke
> > UB if a is 1 - the cast would make it -1 and negation of -1 in signed 1-bit
> > invokes UB.
> > So perhaps we should guard this optimization on type having element precision > 1
> > or being unsigned.  Plus the (convert:type @2) didn't make sense, @2 already
> > must have TREE_TYPE type.
> 
> In the light of the PR111668 patch which shows that
> build_nonstandard_integer_type is needed (at least for some signed prec > 1
> BOOLEAN_TYPEs if we use e.g. negation), I've reworked this patch and handled
> the last problematic build_nonstandard_integer_type call in there as well.
> 
> In the x == cstN ? cst4 : cst3 optimization it uses
> build_nonstandard_integer_type solely for BOOLEAN_TYPEs (I really don't see
> why ENUMERAL_TYPEs would be a problem, we treat them in GIMPLE as uselessly
> convertible to same precision/sign INTEGER_TYPEs), for INTEGER_TYPEs it is
> really a no-op (might return a different type, but always INTEGER_TYPE
> with same TYPE_PRECISION same TYPE_UNSIGNED) and for BITINT_TYPE with larger
> precisions really harmful (we shouldn't create large precision
> INTEGER_TYPEs).
> 
> The a?~t:t optimization just omits the negation of a in type for 1-bit
> precision types or any BOOLEAN_TYPEs.  I think that is correct, because
> for both signed and unsigned 1-bit precision type, cast to type of a bool
> value yields already 0, -1 or 0, 1 values and for 1-bit precision negation
> of that is still 0, -1 or 0, 1 (except for invoking sometimes UB).
> And for signed larger precision BOOLEAN_TYPEs I think it is correct as well,
> cast of [0, 1] to type yields 0, -1 and those can be xored with 0 or -1
> to yield the proper result, any other values would be UB.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2023-10-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/111369
> 	* match.pd (x == cstN ? cst4 : cst3): Use
> 	build_nonstandard_integer_type only if type1 is BOOLEAN_TYPE.
> 	Fix comment typo.  Formatting fix.
> 	(a?~t:t -> (-(a))^t): Always convert to type rather
> 	than using build_nonstandard_integer_type.  Perform negation
> 	only if type has precision > 1 and is not signed BOOLEAN_TYPE.
> 
> --- gcc/match.pd.jj	2023-10-03 10:33:30.817614648 +0200
> +++ gcc/match.pd	2023-10-03 11:29:54.089566764 +0200
> @@ -5178,7 +5178,7 @@ (define_operator_list SYNC_FETCH_AND_AND
>  
>  /* Optimize
>     # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
> -   x_5 ? cstN ? cst4 : cst3
> +   x_5 == cstN ? cst4 : cst3
>     # op is == or != and N is 1 or 2
>     to r_6 = x_5 + (min (cst3, cst4) - cst1) or
>     r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
> @@ -5214,7 +5214,8 @@ (define_operator_list SYNC_FETCH_AND_AND
>  	 type1 = type;
>         auto prec = TYPE_PRECISION (type1);
>         auto unsign = TYPE_UNSIGNED (type1);
> -       type1 = build_nonstandard_integer_type (prec, unsign);
> +       if (TREE_CODE (type1) == BOOLEAN_TYPE)
> +	type1 = build_nonstandard_integer_type (prec, unsign);
>         min = wide_int::from (min, prec,
>  			     TYPE_SIGN (TREE_TYPE (@0)));
>         wide_int a = wide_int::from (wi::to_wide (arg0), prec,
> @@ -5253,14 +5254,7 @@ (define_operator_list SYNC_FETCH_AND_AND
>        }
>        (if (code == PLUS_EXPR)
>         (convert (plus (convert:type1 @0) { arg; }))
> -       (convert (minus { arg; } (convert:type1 @0)))
> -      )
> -     )
> -    )
> -   )
> -  )
> - )
> -)
> +       (convert (minus { arg; } (convert:type1 @0))))))))))
>  #endif
>  
>  (simplify
> @@ -6758,13 +6752,11 @@ (define_operator_list SYNC_FETCH_AND_AND
>   (with { bool wascmp; }
>    (if (INTEGRAL_TYPE_P (type)
>         && bitwise_inverted_equal_p (@1, @2, wascmp)
> -       && (!wascmp || element_precision (type) == 1))
> -   (with {
> -     auto prec = TYPE_PRECISION (type);
> -     auto unsign = TYPE_UNSIGNED (type);
> -     tree inttype = build_nonstandard_integer_type (prec, unsign);
> -    }
> -    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
> +       && (!wascmp || TYPE_PRECISION (type) == 1))
> +   (if ((!TYPE_UNSIGNED (type) && TREE_CODE (type) == BOOLEAN_TYPE)
> +	|| TYPE_PRECISION (type) == 1)
> +    (bit_xor (convert:type @0) @2)
> +    (bit_xor (negate (convert:type @0)) @2)))))
>  #endif
>  
>  /* Simplify pointer equality compares using PTA.  */
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

[Richard Biener]

On Sat, 30 Sep 2023, Jakub Jelinek wrote:

> On Sat, Sep 30, 2023 at 11:44:59AM +0200, Jakub Jelinek wrote:
> > I really can't figure out why one would need to add extra casts.
> > type must be an integral type which has BIT_NOT_EXPR applied on it
> > which yields all ones and we need a type in which negating 0 or 1
> > range will yield 0 or all ones, I think all integral types satisfy
> > that.
> > This fixes PR111369, where one of the bitint*.c tests FAILs with
> > GCC_TEST_RUN_EXPENSIVE=1.
> 
> Though, I think there is an preexisting issue which the
> build_nonstandard_integer_type didn't help with; if type is signed 1-bit
> precision, then I think a ? ~t : t could be valid, but -(type)a would invoke
> UB if a is 1 - the cast would make it -1 and negation of -1 in signed 1-bit
> invokes UB.
> So perhaps we should guard this optimization on type having element precision > 1
> or being unsigned.  Plus the (convert:type @2) didn't make sense, @2 already
> must have TREE_TYPE type.

Alternatively cast to unsigned:1 in that case?  OTOH when element 
precision is 1 we can also simply elide the negation?

> So untested patch would be then:
> 
> 2023-09-29  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/111369
> 	* match.pd (a?~t:t -> (-(a))^t): Always convert to type rather
> 	than using build_nonstandard_integer_type.  Punt if type is
> 	signed 1-bit precision type.
> 
> --- gcc/match.pd.jj	2023-09-29 18:58:42.724956659 +0200
> +++ gcc/match.pd	2023-09-30 11:54:16.603280666 +0200
> @@ -6741,13 +6741,9 @@ (define_operator_list SYNC_FETCH_AND_AND
>   (with { bool wascmp; }
>    (if (INTEGRAL_TYPE_P (type)
>         && bitwise_inverted_equal_p (@1, @2, wascmp)
> -       && (!wascmp || element_precision (type) == 1))
> -   (with {
> -     auto prec = TYPE_PRECISION (type);
> -     auto unsign = TYPE_UNSIGNED (type);
> -     tree inttype = build_nonstandard_integer_type (prec, unsign);
> -    }
> -    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
> +       && (!wascmp || element_precision (type) == 1)
> +       && (!TYPE_OVERFLOW_WRAPS (type) || element_precision (type) > 1))
> +   (bit_xor (negate (convert:type @0)) @2))))
>  #endif
>  
>  /* Simplify pointer equality compares using PTA.  */
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

[Richard Biener]

On Sat, 30 Sep 2023, Jakub Jelinek wrote:

> Hi!
> 
> I really can't figure out why one would need to add extra casts.
> type must be an integral type which has BIT_NOT_EXPR applied on it
> which yields all ones and we need a type in which negating 0 or 1
> range will yield 0 or all ones, I think all integral types satisfy
> that.

It seems to work for bool, so indeed.

> This fixes PR111369, where one of the bitint*.c tests FAILs with
> GCC_TEST_RUN_EXPENSIVE=1.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2023-09-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/111369
> 	* match.pd (a?~t:t -> (-(a))^t): Always convert to type rather
> 	than using build_nonstandard_integer_type.
> 
> --- gcc/match.pd.jj	2023-09-28 11:32:16.122434235 +0200
> +++ gcc/match.pd	2023-09-29 18:05:50.554640268 +0200
> @@ -6742,12 +6742,7 @@ (define_operator_list SYNC_FETCH_AND_AND
>    (if (INTEGRAL_TYPE_P (type)
>         && bitwise_inverted_equal_p (@1, @2, wascmp)
>         && (!wascmp || element_precision (type) == 1))
> -   (with {
> -     auto prec = TYPE_PRECISION (type);
> -     auto unsign = TYPE_UNSIGNED (type);
> -     tree inttype = build_nonstandard_integer_type (prec, unsign);
> -    }
> -    (convert (bit_xor (negate (convert:inttype @0)) (convert:inttype @2)))))))
> +   (bit_xor (negate (convert:type @0)) (convert:type @2)))))
>  #endif
>  
>  /* Simplify pointer equality compares using PTA.  */
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)