Re: [PATCH] match.pd: Some build_nonstandard_integer_type tweaks

[Richard Sandiford <richard.sandiford@arm.com>]

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