match.pd: Some build_nonstandard_integer_type tweaks

  Hi!

As discussed earlier, using build_nonstandard_integer_type blindly for all
INTEGRAL_TYPE_Ps is problematic now that we have BITINT_TYPE, because it
always creates an INTEGRAL_TYPE with some possibly very large precision.
The following patch attempts to deal with 3 such spots in match.pd, others
still need looking at.

In the first case, I think it is quite expensive/undesirable to create
a non-standard INTEGER_TYPE with possibly huge precision and then
immediately just see type_has_mode_precision_p being false for it, or even
worse introducing a cast to TImode or OImode or XImode INTEGER_TYPE which
nothing will be able to actually handle.  128-bit or 64-bit (on 32-bit
targets) types are the largest supported by the backend, so the following
patch avoids creating and matching conversions to larger types, it is
an optimization anyway and so should be used when it is cheap that way.

In the second hunk, I believe the uses of build_nonstandard_integer_type
aren't useful at all.  It is when matching a ? -1 : 0 and trying to express
it as say -(type) (bool) a etc., but this is all GIMPLE only, where most of
integral types with same precision/signedness are compatible and we know
-1 is representable in that type, so I really don't see any reason not to
perform the negation of a [0, 1] valued expression in type, rather
than doing it in
build_nonstandard_integer_type (TYPE_PRECISION (type), TYPE_UNSIGNED (type))
(except that it breaks the BITINT_TYPEs).  I don't think we need to do
something like range_check_type.
While in there, I've also noticed it was using a (with {
tree booltrue = constant_boolean_node (true, boolean_type_node);
} and removed that + replaced uses of booltrue with boolean_true_node
which the above function always returns.

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

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

	* match.pd ((x << c) >> c): Don't call build_nonstandard_integer_type
	nor check type_has_mode_precision_p for width larger than [TD]Imode
	precision.
	(a ? CST1 : CST2): Don't use build_nonstandard_type, just convert
	to type.  Use boolean_true_node instead of
	constant_boolean_node (true, boolean_type_node).  Formatting fixes.


	Jakub
  

On Tue, 19 Sep 2023, Jakub Jelinek wrote:

> Hi!
> 
> As discussed earlier, using build_nonstandard_integer_type blindly for all
> INTEGRAL_TYPE_Ps is problematic now that we have BITINT_TYPE, because it
> always creates an INTEGRAL_TYPE with some possibly very large precision.
> The following patch attempts to deal with 3 such spots in match.pd, others
> still need looking at.
> 
> In the first case, I think it is quite expensive/undesirable to create
> a non-standard INTEGER_TYPE with possibly huge precision and then
> immediately just see type_has_mode_precision_p being false for it, or even
> worse introducing a cast to TImode or OImode or XImode INTEGER_TYPE which
> nothing will be able to actually handle.  128-bit or 64-bit (on 32-bit
> targets) types are the largest supported by the backend, so the following
> patch avoids creating and matching conversions to larger types, it is
> an optimization anyway and so should be used when it is cheap that way.
> 
> In the second hunk, I believe the uses of build_nonstandard_integer_type
> aren't useful at all.  It is when matching a ? -1 : 0 and trying to express
> it as say -(type) (bool) a etc., but this is all GIMPLE only, where most of
> integral types with same precision/signedness are compatible and we know
> -1 is representable in that type, so I really don't see any reason not to
> perform the negation of a [0, 1] valued expression in type, rather
> than doing it in
> build_nonstandard_integer_type (TYPE_PRECISION (type), TYPE_UNSIGNED (type))
> (except that it breaks the BITINT_TYPEs).  I don't think we need to do
> something like range_check_type.
> While in there, I've also noticed it was using a (with {
> tree booltrue = constant_boolean_node (true, boolean_type_node);
> } and removed that + replaced uses of booltrue with boolean_true_node
> which the above function always returns.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2023-09-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* match.pd ((x << c) >> c): Don't call build_nonstandard_integer_type
> 	nor check type_has_mode_precision_p for width larger than [TD]Imode
> 	precision.
> 	(a ? CST1 : CST2): Don't use build_nonstandard_type, just convert
> 	to type.  Use boolean_true_node instead of
> 	constant_boolean_node (true, boolean_type_node).  Formatting fixes.
> 
> --- gcc/match.pd.jj	2023-09-18 10:37:56.002965361 +0200
> +++ gcc/match.pd	2023-09-18 12:14:32.321631010 +0200
> @@ -4114,9 +4114,13 @@ (define_operator_list SYNC_FETCH_AND_AND
>     (if (INTEGRAL_TYPE_P (type))
>      (with {
>        int width = element_precision (type) - tree_to_uhwi (@1);
> -      tree stype = build_nonstandard_integer_type (width, 0);
> +      tree stype = NULL_TREE;
> +      scalar_int_mode mode = (targetm.scalar_mode_supported_p (TImode)
> +			      ? TImode : DImode);
> +      if (width <= GET_MODE_PRECISION (mode))
> +	stype = build_nonstandard_integer_type (width, 0);
>       }
> -     (if (width == 1 || type_has_mode_precision_p (stype))
> +     (if (stype && (width == 1 || type_has_mode_precision_p (stype)))
>        (convert (convert:stype @0))))))))
>  
>  /* Optimize x >> x into 0 */
> @@ -5092,49 +5096,24 @@ (define_operator_list SYNC_FETCH_AND_AND
>      /* a ? -1 : 0 -> -a.  No need to check the TYPE_PRECISION not being 1
>         here as the powerof2cst case above will handle that case correctly.  */
>      (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> +     (negate (convert:type (convert:boolean_type_node @0))))))
> +  (if (integer_zerop (@1))
> +   (switch
> +    /* a ? 0 : 1 -> !a. */
> +    (if (integer_onep (@2))
> +     (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; })))
> +    /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> +    (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
>       (with {
> -       auto prec = TYPE_PRECISION (type);
> -       auto unsign = TYPE_UNSIGNED (type);
> -       tree inttype = build_nonstandard_integer_type (prec, unsign);
> +       tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
>        }
> -      (convert (negate (convert:inttype (convert:boolean_type_node @0))))))))
> -  (if (integer_zerop (@1))
> -   (with {
> -      tree booltrue = constant_boolean_node (true, boolean_type_node);
> -    }
> -    (switch
> -     /* a ? 0 : 1 -> !a. */
> -     (if (integer_onep (@2))
> -      (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
> -     /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> -     (if (INTEGRAL_TYPE_P (type) &&  integer_pow2p (@2))
> -      (with {
> -	tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
> -       }
> -       (lshift (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } ))
> -        { shift; })))
> -     /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
> +      (lshift (convert (bit_xor (convert:boolean_type_node @0)
> +				{ boolean_true_node; })) { shift; })))
> +    /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
>         here as the powerof2cst case above will handle that case correctly.  */
> -     (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> -      (with {
> -	auto prec = TYPE_PRECISION (type);
> -	auto unsign = TYPE_UNSIGNED (type);
> -	tree inttype = build_nonstandard_integer_type (prec, unsign);
> -       }
> -       (convert
> -	(negate
> -         (convert:inttype
> -	  (bit_xor (convert:boolean_type_node @0) { booltrue; } )
> -	 )
> -	)
> -       )
> -      )
> -     )
> -    )
> -   )
> -  )
> - )
> -)
> +    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> +     (negate (convert:type (bit_xor (convert:boolean_type_node @0)
> +				    { boolean_true_node; }))))))))
>  
>  /* (a > 1) ? 0 : (cast)a is the same as (cast)(a == 1)
>     for unsigned types. */
> 
> 	Jakub
> 
>
  

Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi!
>
> As discussed earlier, using build_nonstandard_integer_type blindly for all
> INTEGRAL_TYPE_Ps is problematic now that we have BITINT_TYPE, because it
> always creates an INTEGRAL_TYPE with some possibly very large precision.
> The following patch attempts to deal with 3 such spots in match.pd, others
> still need looking at.
>
> In the first case, I think it is quite expensive/undesirable to create
> a non-standard INTEGER_TYPE with possibly huge precision and then
> immediately just see type_has_mode_precision_p being false for it, or even
> worse introducing a cast to TImode or OImode or XImode INTEGER_TYPE which
> nothing will be able to actually handle.  128-bit or 64-bit (on 32-bit
> targets) types are the largest supported by the backend, so the following
> patch avoids creating and matching conversions to larger types, it is
> an optimization anyway and so should be used when it is cheap that way.
>
> In the second hunk, I believe the uses of build_nonstandard_integer_type
> aren't useful at all.  It is when matching a ? -1 : 0 and trying to express
> it as say -(type) (bool) a etc., but this is all GIMPLE only, where most of
> integral types with same precision/signedness are compatible and we know
> -1 is representable in that type, so I really don't see any reason not to
> perform the negation of a [0, 1] valued expression in type, rather
> than doing it in
> build_nonstandard_integer_type (TYPE_PRECISION (type), TYPE_UNSIGNED (type))
> (except that it breaks the BITINT_TYPEs).  I don't think we need to do
> something like range_check_type.
> While in there, I've also noticed it was using a (with {
> tree booltrue = constant_boolean_node (true, boolean_type_node);
> } and removed that + replaced uses of booltrue with boolean_true_node
> which the above function always returns.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-09-19  Jakub Jelinek  <jakub@redhat.com>
>
> 	* match.pd ((x << c) >> c): Don't call build_nonstandard_integer_type
> 	nor check type_has_mode_precision_p for width larger than [TD]Imode
> 	precision.
> 	(a ? CST1 : CST2): Don't use build_nonstandard_type, just convert
> 	to type.  Use boolean_true_node instead of
> 	constant_boolean_node (true, boolean_type_node).  Formatting fixes.
>
> --- gcc/match.pd.jj	2023-09-18 10:37:56.002965361 +0200
> +++ gcc/match.pd	2023-09-18 12:14:32.321631010 +0200
> @@ -4114,9 +4114,13 @@ (define_operator_list SYNC_FETCH_AND_AND
>     (if (INTEGRAL_TYPE_P (type))
>      (with {
>        int width = element_precision (type) - tree_to_uhwi (@1);
> -      tree stype = build_nonstandard_integer_type (width, 0);
> +      tree stype = NULL_TREE;
> +      scalar_int_mode mode = (targetm.scalar_mode_supported_p (TImode)
> +			      ? TImode : DImode);
> +      if (width <= GET_MODE_PRECISION (mode))
> +	stype = build_nonstandard_integer_type (width, 0);

How about using MAX_FIXED_MODE_SIZE for things like this?

Thanks,
Richard

>       }
> -     (if (width == 1 || type_has_mode_precision_p (stype))
> +     (if (stype && (width == 1 || type_has_mode_precision_p (stype)))
>        (convert (convert:stype @0))))))))
>  
>  /* Optimize x >> x into 0 */
> @@ -5092,49 +5096,24 @@ (define_operator_list SYNC_FETCH_AND_AND
>      /* a ? -1 : 0 -> -a.  No need to check the TYPE_PRECISION not being 1
>         here as the powerof2cst case above will handle that case correctly.  */
>      (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
> +     (negate (convert:type (convert:boolean_type_node @0))))))
> +  (if (integer_zerop (@1))
> +   (switch
> +    /* a ? 0 : 1 -> !a. */
> +    (if (integer_onep (@2))
> +     (convert (bit_xor (convert:boolean_type_node @0) { boolean_true_node; })))
> +    /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> +    (if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
>       (with {
> -       auto prec = TYPE_PRECISION (type);
> -       auto unsign = TYPE_UNSIGNED (type);
> -       tree inttype = build_nonstandard_integer_type (prec, unsign);
> +       tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
>        }
> -      (convert (negate (convert:inttype (convert:boolean_type_node @0))))))))
> -  (if (integer_zerop (@1))
> -   (with {
> -      tree booltrue = constant_boolean_node (true, boolean_type_node);
> -    }
> -    (switch
> -     /* a ? 0 : 1 -> !a. */
> -     (if (integer_onep (@2))
> -      (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
> -     /* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
> -     (if (INTEGRAL_TYPE_P (type) &&  integer_pow2p (@2))
> -      (with {
> -	tree shift = build_int_cst (integer_type_node, tree_log2 (@2));
> -       }
> -       (lshift (convert (bit_xor (convert:boolean_type_node @0) { booltrue; } ))
> -        { shift; })))
> -     /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
> +      (lshift (convert (bit_xor (convert:boolean_type_node @0)
> +				{ boolean_true_node; })) { shift; })))
> +    /* a ? -1 : 0 -> -(!a).  No need to check the TYPE_PRECISION not being 1
>         here as the powerof2cst case above will handle that case correctly.  */
> -     (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> -      (with {
> -	auto prec = TYPE_PRECISION (type);
> -	auto unsign = TYPE_UNSIGNED (type);
> -	tree inttype = build_nonstandard_integer_type (prec, unsign);
> -       }
> -       (convert
> -	(negate
> -         (convert:inttype
> -	  (bit_xor (convert:boolean_type_node @0) { booltrue; } )
> -	 )
> -	)
> -       )
> -      )
> -     )
> -    )
> -   )
> -  )
> - )
> -)
> +    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
> +     (negate (convert:type (bit_xor (convert:boolean_type_node @0)
> +				    { boolean_true_node; }))))))))
>  
>  /* (a > 1) ? 0 : (cast)a is the same as (cast)(a == 1)
>     for unsigned types. */
>
> 	Jakub