Add a full_integral_type_p helper function

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Aug 18, 2017 at 1:04 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Fri, Aug 18, 2017 at 10:10 AM, Richard Sandiford
>>> <richard.sandiford@linaro.org> wrote:
>>>> There are several places that test whether:
>>>>
>>>>     TYPE_PRECISION (t) == GET_MODE_PRECISION (TYPE_MODE (t))
>>>>
>>>> for some integer type T.  With SVE variable-length modes, this would
>>>> need to become:
>>>>
>>>>     TYPE_PRECISION (t) == GET_MODE_PRECISION (SCALAR_TYPE_MODE (t))
>>>>
>>>> (or SCALAR_INT_TYPE_MODE, it doesn't matter which in this case).
>>>> But rather than add the "SCALAR_" everywhere, it seemed neater to
>>>> introduce a new helper function that tests whether T is an integral
>>>> type that has the same number of bits as its underlying mode.  This
>>>> patch does that, calling it full_integral_type_p.
>>>>
>>>> It isn't possible to use TYPE_MODE in tree.h because vector_type_mode
>>>> is defined in stor-layout.h, so for now the function accesses the mode
>>>> field directly.  After the 77-patch machine_mode series (thanks again
>>>> Jeff for the reviews) it would use SCALAR_TYPE_MODE instead.
>>>>
>>>> Of the changes that didn't previously have an INTEGRAL_TYPE_P check:
>>>>
>>>> - for fold_single_bit_test_into_sign_test it is obvious from the
>>>>   integer_foop tests that this is restricted to integral types.
>>>>
>>>> - vect_recog_vector_vector_shift_pattern is inherently restricted
>>>>   to integral types.
>>>>
>>>> - the register_edge_assert_for_2 hunk is dominated by:
>>>>
>>>>       TREE_CODE (val) == INTEGER_CST
>>>>
>>>> - the ubsan_instrument_shift hunk is preceded by an early exit:
>>>>
>>>>       if (!INTEGRAL_TYPE_P (type0))
>>>>         return NULL_TREE;
>>>>
>>>> - the second and third match.pd hunks are from:
>>>>
>>>>     /* Fold (X << C1) & C2 into (X << C1) & (C2 | ((1 << C1) - 1))
>>>>             (X >> C1) & C2 into (X >> C1) & (C2 | ~((type) -1 >> C1))
>>>>        if the new mask might be further optimized.  */
>>>>
>>>> I'm a bit confused about:
>>>>
>>>> /* Try to fold (type) X op CST -> (type) (X op ((type-x) CST))
>>>>    when profitable.
>>>>    For bitwise binary operations apply operand conversions to the
>>>>    binary operation result instead of to the operands.  This allows
>>>>    to combine successive conversions and bitwise binary operations.
>>>>    We combine the above two cases by using a conditional convert.  */
>>>> (for bitop (bit_and bit_ior bit_xor)
>>>>  (simplify
>>>>   (bitop (convert @0) (convert? @1))
>>>>   (if (((TREE_CODE (@1) == INTEGER_CST
>>>>          && INTEGRAL_TYPE_P (TREE_TYPE (@0))
>>>>          && int_fits_type_p (@1, TREE_TYPE (@0)))
>>>>         || types_match (@0, @1))
>>>>        /* ???  This transform conflicts with fold-const.c doing
>>>>           Convert (T)(x & c) into (T)x & (T)c, if c is an integer
>>>>           constants (if x has signed type, the sign bit cannot be set
>>>>           in c).  This folds extension into the BIT_AND_EXPR.
>>>>           Restrict it to GIMPLE to avoid endless recursions.  */
>>>>        && (bitop != BIT_AND_EXPR || GIMPLE)
>>>>        && (/* That's a good idea if the conversion widens the operand, thus
>>>> after hoisting the conversion the operation will be narrower.  */
>>>>            TYPE_PRECISION (TREE_TYPE (@0)) < TYPE_PRECISION (type)
>>>>            /* It's also a good idea if the conversion is to a non-integer
>>>>               mode.  */
>>>>            || GET_MODE_CLASS (TYPE_MODE (type)) != MODE_INT
>>>>            /* Or if the precision of TO is not the same as the precision
>>>>               of its mode.  */
>>>> || TYPE_PRECISION (type) != GET_MODE_PRECISION (TYPE_MODE (type))))
>>>>    (convert (bitop @0 (convert @1))))))
>>>>
>>>> though.  The "INTEGRAL_TYPE_P (TREE_TYPE (@0))" suggests that we can't
>>>> rely on @0 and @1 being integral (although conversions from float would
>>>> use FLOAT_EXPR), but then what is:
>>>
>>> bit_and is valid on POINTER_TYPE and vector integer types
>>>
>>>>
>>>>            /* It's also a good idea if the conversion is to a non-integer
>>>>               mode.  */
>>>>            || GET_MODE_CLASS (TYPE_MODE (type)) != MODE_INT
>>>>
>>>> letting through?  MODE_PARTIAL_INT maybe, but that's a sort of integer
>>>> mode too.  MODE_COMPLEX_INT or MODE_VECTOR_INT?  I thought for those
>>>> it would be better to apply the scalar rules to the element type.
>>>
>>> I suppose extra caution ;)  I think I have seen BLKmode for not naturally
>>> aligned integer types at least on strict-align targets?  The code is
>>> a copy from original code in tree-ssa-forwprop.c.
>>>
>>>> Either way, having allowed all non-INT modes, using full_integral_type_p
>>>> for the remaining condition seems correct.
>>>>
>>>> If the feeling is that this isn't a useful abstraction, I can just update
>>>> each site individually to cope with variable-sized modes.
>>>
>>> I think "full_integral_type_p" is a name from which I cannot infer
>>> its meaning.  Maybe type_has_mode_precision_p?  Or
>>> type_matches_mode_p?
>>
>> type_has_mode_precision_p sounds good.  With that name I guess it
>> should be written to cope with all types (even those with variable-
>> width modes), so I think we'd need to continue using TYPE_MODE.
>> The VECTOR_MODE_P check should get optimised away in most of
>> the cases touched by the patch though.
>>
>> Would it be OK to move the declaration of vector_type_mode to tree.h
>> so that type_has_mode_precision_p can be inline?
>
> Just move the implementation to tree.c then.

OK, I posted that as: https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01184.html

Here's the patch to add type_has_mode_precision_p.  Tested on
aarch64-linux-gnu and x86_64-linux-gnu, and by diffing the before and
after testsuite assembly for one target per CPU (there were no differences).
OK to install?

Thanks,
Richard

2017-08-21  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* tree.h (type_has_mode_precision_p): New function.
	* convert.c (convert_to_integer_1): Use it.
	* expr.c (expand_expr_real_2): Likewise.
	(expand_expr_real_1): Likewise.
	* fold-const.c (fold_single_bit_test_into_sign_test): Likewise.
	* match.pd: Likewise.
	* tree-ssa-forwprop.c (simplify_rotate): Likewise.
	* tree-ssa-math-opts.c (convert_mult_to_fma): Likewise.
	* tree-tailcall.c (process_assignment): Likewise.
	* tree-vect-loop.c (vectorizable_reduction): Likewise.
	* tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern)
	(vect_recog_mult_pattern, vect_recog_divmod_pattern): Likewise.
	* tree-vect-stmts.c (vectorizable_conversion): Likewise.
	(vectorizable_assignment): Likewise.
	(vectorizable_shift): Likewise.
	(vectorizable_operation): Likewise.
	* tree-vrp.c (register_edge_assert_for_2): Likewise.

Add a full_integral_type_p helper function

Commit Message

Comments

Patch