[005/nnn] poly_int: rtx constants
Jeff Law
law@redhat.com
Tue Dec 19 04:52:00 GMT 2017
On 12/14/2017 06:25 PM, Richard Sandiford wrote:
> Jeff Law <law@redhat.com> writes:
>> On 10/23/2017 11:00 AM, Richard Sandiford wrote:
>>> This patch adds an rtl representation of poly_int values.
>>> There were three possible ways of doing this:
>>>
>>> (1) Add a new rtl code for the poly_ints themselves and store the
>>> coefficients as trailing wide_ints. This would give constants like:
>>>
>>> (const_poly_int [c0 c1 ... cn])
>>>
>>> The runtime value would be:
>>>
>>> c0 + c1 * x1 + ... + cn * xn
>>>
>>> (2) Like (1), but use rtxes for the coefficients. This would give
>>> constants like:
>>>
>>> (const_poly_int [(const_int c0)
>>> (const_int c1)
>>> ...
>>> (const_int cn)])
>>>
>>> although the coefficients could be const_wide_ints instead
>>> of const_ints where appropriate.
>>>
>>> (3) Add a new rtl code for the polynomial indeterminates,
>>> then use them in const wrappers. A constant like c0 + c1 * x1
>>> would then look like:
>>>
>>> (const:M (plus:M (mult:M (const_param:M x1)
>>> (const_int c1))
>>> (const_int c0)))
>>>
>>> There didn't seem to be that much to choose between them. The main
>>> advantage of (1) is that it's a more efficient representation and
>>> that we can refer to the cofficients directly as wide_int_storage.
>> Well, and #1 feels more like how we handle CONST_INT :-)
>>>
>>>
>>> 2017-10-23 Richard Sandiford <richard.sandiford@linaro.org>
>>> Alan Hayward <alan.hayward@arm.com>
>>> David Sherwood <david.sherwood@arm.com>
>>>
>>> gcc/
>>> * doc/rtl.texi (const_poly_int): Document.
>>> * gengenrtl.c (excluded_rtx): Return true for CONST_POLY_INT.
>>> * rtl.h (const_poly_int_def): New struct.
>>> (rtx_def::u): Add a cpi field.
>>> (CASE_CONST_UNIQUE, CASE_CONST_ANY): Add CONST_POLY_INT.
>>> (CONST_POLY_INT_P, CONST_POLY_INT_COEFFS): New macros.
>>> (wi::rtx_to_poly_wide_ref): New typedef
>>> (const_poly_int_value, wi::to_poly_wide, rtx_to_poly_int64)
>>> (poly_int_rtx_p): New functions.
>>> (trunc_int_for_mode): Declare a poly_int64 version.
>>> (plus_constant): Take a poly_int64 instead of a HOST_WIDE_INT.
>>> (immed_wide_int_const): Take a poly_wide_int_ref rather than
>>> a wide_int_ref.
>>> (strip_offset): Declare.
>>> (strip_offset_and_add): New function.
>>> * rtl.def (CONST_POLY_INT): New rtx code.
>>> * rtl.c (rtx_size): Handle CONST_POLY_INT.
>>> (shared_const_p): Use poly_int_rtx_p.
>>> * emit-rtl.h (gen_int_mode): Take a poly_int64 instead of a
>>> HOST_WIDE_INT.
>>> (gen_int_shift_amount): Likewise.
>>> * emit-rtl.c (const_poly_int_hasher): New class.
>>> (const_poly_int_htab): New variable.
>>> (init_emit_once): Initialize it when NUM_POLY_INT_COEFFS > 1.
>>> (const_poly_int_hasher::hash): New function.
>>> (const_poly_int_hasher::equal): Likewise.
>>> (gen_int_mode): Take a poly_int64 instead of a HOST_WIDE_INT.
>>> (immed_wide_int_const): Rename to...
>>> (immed_wide_int_const_1): ...this and make static.
>>> (immed_wide_int_const): New function, taking a poly_wide_int_ref
>>> instead of a wide_int_ref.
>>> (gen_int_shift_amount): Take a poly_int64 instead of a HOST_WIDE_INT.
>>> (gen_lowpart_common): Handle CONST_POLY_INT.
>>> * cse.c (hash_rtx_cb, equiv_constant): Likewise.
>>> * cselib.c (cselib_hash_rtx): Likewise.
>>> * dwarf2out.c (const_ok_for_output_1): Likewise.
>>> * expr.c (convert_modes): Likewise.
>>> * print-rtl.c (rtx_writer::print_rtx, print_value): Likewise.
>>> * rtlhash.c (add_rtx): Likewise.
>>> * explow.c (trunc_int_for_mode): Add a poly_int64 version.
>>> (plus_constant): Take a poly_int64 instead of a HOST_WIDE_INT.
>>> Handle existing CONST_POLY_INT rtxes.
>>> * expmed.h (expand_shift): Take a poly_int64 instead of a
>>> HOST_WIDE_INT.
>>> * expmed.c (expand_shift): Likewise.
>>> * rtlanal.c (strip_offset): New function.
>>> (commutative_operand_precedence): Give CONST_POLY_INT the same
>>> precedence as CONST_DOUBLE and put CONST_WIDE_INT between that
>>> and CONST_INT.
>>> * rtl-tests.c (const_poly_int_tests): New struct.
>>> (rtl_tests_c_tests): Use it.
>>> * simplify-rtx.c (simplify_const_unary_operation): Handle
>>> CONST_POLY_INT.
>>> (simplify_const_binary_operation): Likewise.
>>> (simplify_binary_operation_1): Fold additions of symbolic constants
>>> and CONST_POLY_INTs.
>>> (simplify_subreg): Handle extensions and truncations of
>>> CONST_POLY_INTs.
>>> (simplify_const_poly_int_tests): New struct.
>>> (simplify_rtx_c_tests): Use it.
>>> * wide-int.h (storage_ref): Add default constructor.
>>> (wide_int_ref_storage): Likewise.
>>> (trailing_wide_ints): Use GTY((user)).
>>> (trailing_wide_ints::operator[]): Add a const version.
>>> (trailing_wide_ints::get_precision): New function.
>>> (trailing_wide_ints::extra_size): Likewise.
>> Do we need to define anything WRT structure sharing in rtl.texi for a
>> CONST_POLY_INT?
>
> Good catch. Fixed in the patch below.
>
>>> Index: gcc/rtl.c
>>> ===================================================================
>>> --- gcc/rtl.c 2017-10-23 16:52:20.579835373 +0100
>>> +++ gcc/rtl.c 2017-10-23 17:00:54.443002147 +0100
>>> @@ -257,9 +261,10 @@ shared_const_p (const_rtx orig)
>>>
>>> /* CONST can be shared if it contains a SYMBOL_REF. If it contains
>>> a LABEL_REF, it isn't sharable. */
>>> + poly_int64 offset;
>>> return (GET_CODE (XEXP (orig, 0)) == PLUS
>>> && GET_CODE (XEXP (XEXP (orig, 0), 0)) == SYMBOL_REF
>>> - && CONST_INT_P (XEXP (XEXP (orig, 0), 1)));
>>> + && poly_int_rtx_p (XEXP (XEXP (orig, 0), 1), &offset));
>> Did this just change structure sharing for CONST_WIDE_INT?
>
> No, we'd only use CONST_WIDE_INT for things that don't fit in
> poly_int64.
>
>>> + /* Create a new rtx. There's a choice to be made here between installing
>>> + the actual mode of the rtx or leaving it as VOIDmode (for consistency
>>> + with CONST_INT). In practice the handling of the codes is different
>>> + enough that we get no benefit from using VOIDmode, and various places
>>> + assume that VOIDmode implies CONST_INT. Using the real mode seems like
>>> + the right long-term direction anyway. */
>> Certainly my preference is to get the mode in there. I see modeless
>> CONST_INTs as a long standing wart and I'm not keen to repeat it.
>
> Yeah. Still regularly hit problems related to modeless CONST_INTs
> today (including the gen_int_shift_amount patch).
>
>>> Index: gcc/wide-int.h
>>> ===================================================================
>>> --- gcc/wide-int.h 2017-10-23 17:00:20.923835582 +0100
>>> +++ gcc/wide-int.h 2017-10-23 17:00:54.445999420 +0100
>>> @@ -613,6 +613,7 @@ #define SHIFT_FUNCTION \
>>> access. */
>>> struct storage_ref
>>> {
>>> + storage_ref () {}
>>> storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int);
>>>
>>> const HOST_WIDE_INT *val;
>>> @@ -944,6 +945,8 @@ struct wide_int_ref_storage : public wi:
>>> HOST_WIDE_INT scratch[2];
>>>
>>> public:
>>> + wide_int_ref_storage () {}
>>> +
>>> wide_int_ref_storage (const wi::storage_ref &);
>>>
>>> template <typename T>
>> So doesn't this play into the whole question about initialization of
>> these objects. So I'll defer on this hunk until we settle that
>> question, but the rest is OK.
>
> Any more thoughts on this? In the end the 001 patch went in with
> the empty constructors. Like I say, I'm happy to switch to C++-11
> "= default;" once we require C++11, but I think having well-defined
> implicit construction would make switching to "= default" harder
> in future.
I think we're good to go. I would have slightly preferred to avoid the
empty ctor, but not enough to raise an objection to Richi's ACK and
ultimately make the switch to = default harder later.
And just to be clear, I'd like to propose we step forward to C++11 in
the gcc-9 timeframe. I haven't run that by anyone, but that's the
timeframe I'd personally prefer.
jeff
More information about the Gcc-patches
mailing list