Re: [005/nnn] poly_int: rtx constants

[Jeff Law <law@redhat.com>]

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?



>  
> 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?



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



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


Jeff