diff mbox series

match.pd: Avoid another build_nonstandard_integer_type call [PR111369]

Message ID ZRfuG4OTJ0glNRUG@tucnak
State Unresolved
Headers
Series match.pd: Avoid another build_nonstandard_integer_type call [PR111369] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Sept. 30, 2023, 9:44 a.m. UTC 
  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.


	Jakub

Comments

Jakub Jelinek Sept. 30, 2023, 9:57 a.m. UTC | #1 
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
Richard Biener Oct. 4, 2023, 6:34 a.m. UTC | #2 
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 Oct. 4, 2023, 6:36 a.m. UTC | #3 
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
> 
>
diff mbox series

Patch

--- 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.  */