diff mbox series

[6/7] middle-end: add vec_init support for variable length subvector concatenation.

Message ID Z1BIgVXdh83tporj@arm.com
State New
Headers
Series None |

Commit Message

Tamar Christina Dec. 4, 2024, 12:18 p.m. UTC 
  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.christina@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

---




--

Comments

Richard Biener Dec. 4, 2024, 2:53 p.m. UTC | #1 
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.christina@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..8bdec1cbf78ce338c135a6660bcb3abc75884c0c 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?

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);
>  	      }
>  	  }
>  
> 
> 
> 
> 
>
Tamar Christina Dec. 4, 2024, 3:02 p.m. UTC | #2 
> -----Original Message-----
> From: Richard Biener <rguenther@suse.de>
> Sent: Wednesday, December 4, 2024 2:53 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Sandiford
> <Richard.Sandiford@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.christina@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..

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 <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Tamar Christina Dec. 4, 2024, 3:17 p.m. UTC | #3 
-----Original Message-----
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: Wednesday, December 4, 2024 3:02 PM
> To: Richard Biener <rguenther@suse.de>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: RE: [PATCH 6/7]middle-end: add vec_init support for variable length
> subvector concatenation.
> 
> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: Wednesday, December 4, 2024 2:53 PM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Sandiford
> > <Richard.Sandiford@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.christina@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..
> 

Err nunit=8.  But the result of the division between the poly and nunits was another poly.
normally we would skip the entire block for this so I did the same.

Thanks,
Tamar

> 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 <rguenther@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 Dec. 5, 2024, 12:52 p.m. UTC | #4 
On Wed, 4 Dec 2024, Tamar Christina wrote:

> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: Wednesday, December 4, 2024 2:53 PM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Sandiford
> > <Richard.Sandiford@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.christina@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 <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
>
Richard Sandiford Dec. 5, 2024, 3:17 p.m. UTC | #5 
Tamar Christina <Tamar.Christina@arm.com> writes:
>> > @@ -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

JFTR, it was Tejas, not me :)

Richard

>
> That ICEd here because n_elts is a poly [16, 16] and nunits was 1 I think..
>
> Tamar
Tamar Christina Dec. 9, 2024, 1:04 p.m. UTC | #6 
> >> So I think we can simply set const_n_elts to CONSTRUCTOR_NELTS
> >> for vector_typed_elts_p?
> >>
> >

Done,

gcc/ChangeLog:

	PR target/96342
	* expr.cc (store_constructor): add support for variable-length
	vectors.

Co-authored-by: Tamar Christina <tamar.christina@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

-- inline copy of patch --

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 4c6039c6608c0d9db3d1796eeab2129cb844433f..babf00f34dcf1ac81a9d2d9947350fb1c0455811 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -7965,12 +7965,9 @@ 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;
-
+	    const_n_elts = 0;
 	    if (CONSTRUCTOR_NELTS (exp)
 		&& (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)->value))
 		    == VECTOR_TYPE))
@@ -7979,23 +7976,26 @@ 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;
+
+		icode = convert_optab_handler (vec_init_optab, mode,
+					       TYPE_MODE (etype));
+		const_n_elts = CONSTRUCTOR_NELTS (exp);
+		vec_vec_init_p = icode != CODE_FOR_nothing;
 	      }
-	    icode = convert_optab_handler (vec_init_optab, mode, emode);
-	    if (icode != CODE_FOR_nothing)
+	    else if (exact_div (n_elts, GET_MODE_NUNITS (eltmode))
+			.is_constant (&const_n_elts))
 	      {
-		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);
+		/* For a non-const type vector, we check it is made up of
+		   similarly non-const type vectors. */
+		icode = convert_optab_handler (vec_init_optab, mode, eltmode);
 	      }
+
+	  if (const_n_elts && icode != CODE_FOR_nothing)
+	    {
+	      vector = rtvec_alloc (const_n_elts);
+	      for (unsigned int k = 0; k < const_n_elts; k++)
+		RTVEC_ELT (vector, k) = CONST0_RTX (eltmode);
+	    }
 	  }
 
 	/* Compute the size of the elements in the CTOR.  It differs
Richard Biener Dec. 10, 2024, 2:20 p.m. UTC | #7 
On Mon, 9 Dec 2024, Tamar Christina wrote:

> > >> So I think we can simply set const_n_elts to CONSTRUCTOR_NELTS
> > >> for vector_typed_elts_p?
> > >>
> > >
> 
> Done,
> 
> gcc/ChangeLog:
> 
> 	PR target/96342
> 	* expr.cc (store_constructor): add support for variable-length
> 	vectors.
> 
> Co-authored-by: Tamar Christina <tamar.christina@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?

OK unless Richard S. has any comments.

Thanks,
Richard.

> Thanks,
> Tamar
> 
> -- inline copy of patch --
> 
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 4c6039c6608c0d9db3d1796eeab2129cb844433f..babf00f34dcf1ac81a9d2d9947350fb1c0455811 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -7965,12 +7965,9 @@ 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;
> -
> +	    const_n_elts = 0;
>  	    if (CONSTRUCTOR_NELTS (exp)
>  		&& (TREE_CODE (TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)->value))
>  		    == VECTOR_TYPE))
> @@ -7979,23 +7976,26 @@ 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;
> +
> +		icode = convert_optab_handler (vec_init_optab, mode,
> +					       TYPE_MODE (etype));
> +		const_n_elts = CONSTRUCTOR_NELTS (exp);
> +		vec_vec_init_p = icode != CODE_FOR_nothing;
>  	      }
> -	    icode = convert_optab_handler (vec_init_optab, mode, emode);
> -	    if (icode != CODE_FOR_nothing)
> +	    else if (exact_div (n_elts, GET_MODE_NUNITS (eltmode))
> +			.is_constant (&const_n_elts))
>  	      {
> -		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);
> +		/* For a non-const type vector, we check it is made up of
> +		   similarly non-const type vectors. */
> +		icode = convert_optab_handler (vec_init_optab, mode, eltmode);
>  	      }
> +
> +	  if (const_n_elts && icode != CODE_FOR_nothing)
> +	    {
> +	      vector = rtvec_alloc (const_n_elts);
> +	      for (unsigned int k = 0; k < const_n_elts; k++)
> +		RTVEC_ELT (vector, k) = CONST0_RTX (eltmode);
> +	    }
>  	  }
>  
>  	/* Compute the size of the elements in the CTOR.  It differs
>
diff mbox series

Patch

diff --git a/gcc/expr.cc b/gcc/expr.cc
index 2d90d7aac296077cc0bda8a1b4732b1cd44a610d..8bdec1cbf78ce338c135a6660bcb3abc75884c0c 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))
+	      {
+		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);
 	      }
 	  }