On Wed, 4 Dec 2024, Tamar Christina wrote:
> > -----Original Message-----
> > From: Richard Biener <rguent...@suse.de>
> > Sent: Wednesday, December 4, 2024 2:53 PM
> > To: Tamar Christina <tamar.christ...@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Sandiford
> > <richard.sandif...@arm.com>
> > Subject: Re: [PATCH 6/7]middle-end: add vec_init support for variable length
> > subvector concatenation.
> >
> > On Wed, 4 Dec 2024, Tamar Christina wrote:
> >
> > > Hi All,
> > >
> > > For architectures where the vector-length is a compile-time variable,
> > > rather representing a runtime constant, as is the case with SVE it is
> > > perfectly reasonable that such vector be made up of two (or more)
> > > subvector
> > > components of a compatible sub-length variable.
> > >
> > > One example of this would be the concatenation of two VNx4QI vectors
> > > into a single VNx8QI vector.
> > >
> > > This patch adds initial support for the enablement of this feature in
> > > the middle-end, removing the `.is_constant()' constraint on the vector's
> > > number of elements, instead making the constant no. of elements the
> > > multiple of the number of subvectors (which must then also be of
> > > variable length, such that their polynomial ratio then results in a
> > > compile-time constant) required to fill the vector.
> > >
> > > gcc/ChangeLog:
> > >
> > > PR target/96342
> > > * expr.cc (store_constructor): add support for variable-length
> > > vectors.
> > >
> > > 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/expr.cc b/gcc/expr.cc
> > > index
> > 2d90d7aac296077cc0bda8a1b4732b1cd44a610d..8bdec1cbf78ce338c135a666
> > 0bcb3abc75884c0c 100644
> > > --- a/gcc/expr.cc
> > > +++ b/gcc/expr.cc
> > > @@ -7962,11 +7962,11 @@ store_constructor (tree exp, rtx target, int
> > > cleared,
> > poly_int64 size,
> > >
> > > n_elts = TYPE_VECTOR_SUBPARTS (type);
> > > if (REG_P (target)
> > > - && VECTOR_MODE_P (mode)
> > > - && n_elts.is_constant (&const_n_elts))
> > > + && VECTOR_MODE_P (mode))
> > > {
> > > machine_mode emode = eltmode;
> > > bool vector_typed_elts_p = false;
> > > + auto nunits = GET_MODE_NUNITS (emode);
> > >
> > > if (CONSTRUCTOR_NELTS (exp)
> > > && (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)-
> > >value))
> > > @@ -7976,22 +7976,30 @@ store_constructor (tree exp, rtx target, int
> > > cleared,
> > poly_int64 size,
> > > gcc_assert (known_eq (CONSTRUCTOR_NELTS (exp)
> > > * TYPE_VECTOR_SUBPARTS (etype),
> > > n_elts));
> > > +
> > > emode = TYPE_MODE (etype);
> > > vector_typed_elts_p = true;
> > > + nunits = TYPE_VECTOR_SUBPARTS (etype);
> > > }
> > > - icode = convert_optab_handler (vec_init_optab, mode, emode);
> > > - if (icode != CODE_FOR_nothing)
> > > - {
> > > - unsigned int n = const_n_elts;
> > >
> > > - if (vector_typed_elts_p)
> > > + /* For a non-const type vector, we check it is made up of similarly
> > > + non-const type vectors. */
> > > + if (exact_div (n_elts, nunits).is_constant (&const_n_elts))
> >
> > I think this is guaranteed by tree-cfg.cc:4767?
> >
> > So I think we can simply set const_n_elts to CONSTRUCTOR_NELTS
> > for vector_typed_elts_p?
> >
>
> I thought so too.. and then two days ago Ricard S committed this ACLE
> testcase:
> ./gcc/testsuite/gcc.target/aarch64/sve/acle/general/cops.c
>
> That ICEd here because n_elts is a poly [16, 16] and nunits was 1 I think..
In any case the GIMPLE constraints were (supposed to be) set up in a
way that testing the vec_init_optab is always applicable.
Richard.
> Tamar
>
> > That said - by refactoring to separate the vector elt from scalar
> > elt case this might look more obvious (also no need to clear
> > RTVEC_ELT in that case)?
> >
> > > + {
> > > + icode = convert_optab_handler (vec_init_optab, mode, emode);
> > > + if (icode != CODE_FOR_nothing)
> > > {
> > > - n = CONSTRUCTOR_NELTS (exp);
> > > - vec_vec_init_p = true;
> > > + unsigned int n = const_n_elts;
> > > +
> > > + if (vector_typed_elts_p)
> > > + {
> > > + n = CONSTRUCTOR_NELTS (exp);
> > > + vec_vec_init_p = true;
> > > + }
> > > + vector = rtvec_alloc (n);
> > > + for (unsigned int k = 0; k < n; k++)
> > > + RTVEC_ELT (vector, k) = CONST0_RTX (emode);
> > > }
> > > - vector = rtvec_alloc (n);
> > > - for (unsigned int k = 0; k < n; k++)
> > > - RTVEC_ELT (vector, k) = CONST0_RTX (emode);
> > > }
> > > }
> > >
> > >
> > >
> > >
> > >
> > >
> >
> > --
> > Richard Biener <rguent...@suse.de>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
