On Wed, Dec 4, 2024 at 1:21 PM Tamar Christina <tamar.christ...@arm.com> wrote:
>
> Hi All,
>
> While `poly_int64' has been the default representation of bitfield size
> and offset for some time, there was a lack of support for the use of
> non-constant `poly_int64' values for those values throughout the
> compiler, limiting the applicability of the BIT_FIELD_REF rtl expression
> for variable length vectors, such as those used by SVE.
>
> This patch starts work on extending the functionality of relevant
> functions in the expand pass such as to enable their use by the compiler
> for such vectors.
>
> gcc/ChangeLog:
>
> PR target/96342
> * expr.cc (store_constructor): Enable poly_{u}int64 type usage.
> (get_inner_reference): Ditto.
> * expmed.cc (store_bit_field_1): Add is_constant checks to bitsize
> and bitnum.
>
> Co-authored-by: Tamar Christina <tamar.christ...@arm.com>
>
> Bootstrapped Regtested on aarch64-none-linux-gnu,
> arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> -m32, -m64 and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> ---
> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
> index
> 2d5e5243ce8e9c7dd2eeb0996711801d969b3f22..3721d55967ba21516b9a816505e0e999a53c3d2f
> 100644
> --- a/gcc/expmed.cc
> +++ b/gcc/expmed.cc
> @@ -849,6 +849,10 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize,
> poly_uint64 bitnum,
> polynomial bitnum and bitsize. */
>
> /* From here on we need to be looking at a fixed-size insertion. */
> +
> + if (!bitsize.is_constant () || !bitnum.is_constant ())
> + error ("bit field masks of variable size length/offset unsupported");
> +
Nak for this, we'll ICE immediately after anyway.
> unsigned HOST_WIDE_INT ibitsize = bitsize.to_constant ();
> unsigned HOST_WIDE_INT ibitnum = bitnum.to_constant ();
>
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index
> 70f2ecec99839e6247ed7601e0bff67e7aa38ba4..2d90d7aac296077cc0bda8a1b4732b1cd44a610d
> 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -7897,15 +7897,15 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> {
> unsigned HOST_WIDE_INT idx;
> constructor_elt *ce;
> - int i;
> + poly_int64 i;
> bool need_to_clear;
> insn_code icode = CODE_FOR_nothing;
> tree elt;
> tree elttype = TREE_TYPE (type);
> int elt_size = vector_element_bits (type);
> machine_mode eltmode = TYPE_MODE (elttype);
> - HOST_WIDE_INT bitsize;
> - HOST_WIDE_INT bitpos;
> + poly_int64 bitsize;
> + poly_int64 bitpos;
> rtvec vector = NULL;
> poly_uint64 n_elts;
> unsigned HOST_WIDE_INT const_n_elts;
> @@ -8002,7 +8002,7 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> ? TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)->value)
> : elttype);
> if (VECTOR_TYPE_P (val_type))
> - bitsize = tree_to_uhwi (TYPE_SIZE (val_type));
> + bitsize = tree_to_poly_uint64 (TYPE_SIZE (val_type));
> else
> bitsize = elt_size;
>
> @@ -8015,12 +8015,12 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> need_to_clear = true;
> else
> {
> - unsigned HOST_WIDE_INT count = 0, zero_count = 0;
> + poly_uint64 count = 0, zero_count = 0;
> tree value;
>
> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (exp), idx, value)
> {
> - int n_elts_here = bitsize / elt_size;
> + poly_int64 n_elts_here = exact_div (bitsize, elt_size);
> count += n_elts_here;
> if (mostly_zeros_p (value))
> zero_count += n_elts_here;
> @@ -8029,7 +8029,7 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> /* Clear the entire vector first if there are any missing
> elements,
> or if the incidence of zero elements is >= 75%. */
> need_to_clear = (maybe_lt (count, n_elts)
> - || 4 * zero_count >= 3 * count);
> + || maybe_gt (4 * zero_count, 3 * count));
> }
>
> if (need_to_clear && maybe_gt (size, 0) && !vector)
> @@ -8058,9 +8058,9 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> element of TARGET, determined by counting the elements. */
> for (idx = 0, i = 0;
> vec_safe_iterate (CONSTRUCTOR_ELTS (exp), idx, &ce);
> - idx++, i += bitsize / elt_size)
> + idx++, i += exact_div (bitsize, elt_size))
> {
> - HOST_WIDE_INT eltpos;
> + poly_int64 eltpos;
> tree value = ce->value;
That seems incomplete. We also have
if (ce->index)
eltpos = tree_to_uhwi (ce->index);
else
eltpos = i;
which should use tree_to_poly_int64 then? And if .to_constant () below works,
why not use constant_multiple_p instead of exact_div?
It seems to me the code interweaves two cases with multi-exclusive conditions
where only one is expected to have poly-ints but the changes make sure to
obfuscate this even more ...
Note we have GIMPLE verification that vector CTOR have no holes and no
or incrementing INTEGER_CST indices unless the components are vectors
themselves in which case the CTOR has to be full and CTOR_NELTS *
subparts of element
should be known_eq to subparts of the CTOR vector type.
> if (cleared && initializer_zerop (value))
> @@ -8081,7 +8081,7 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> }
> else
> gcc_assert (TREE_CODE (TREE_TYPE (value)) != VECTOR_TYPE);
> - RTVEC_ELT (vector, eltpos) = expand_normal (value);
> + RTVEC_ELT (vector, eltpos.to_constant ()) = expand_normal
> (value);
> }
> else
> {
> @@ -8457,10 +8457,10 @@ get_inner_reference (tree exp, poly_int64 *pbitsize,
>
> if (size_tree != 0)
> {
> - if (! tree_fits_uhwi_p (size_tree))
> - mode = BLKmode, *pbitsize = -1;
> + if (poly_int_tree_p (size_tree, pbitsize))
> + ;
> else
> - *pbitsize = tree_to_uhwi (size_tree);
> + mode = BLKmode, *pbitsize = -1;
> }
>
> *preversep = reverse_storage_order_for_component_p (exp);
>
>
>
>
> --
