On Tue, 29 Jul 2025, Pengfei Li wrote:
> Hi Richi,
>
> > > > > > diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> > > > > > index 75e06ff28e6..8595c76eae2 100644
> > > > > > --- a/gcc/tree-vect-data-refs.cc
> > > > > > +++ b/gcc/tree-vect-data-refs.cc
> > > > > > @@ -2972,7 +2972,8 @@ vect_enhance_data_refs_alignment
> > > > > > (loop_vec_info
> > > > loop_vinfo)
> > > > > > VF is a power of two. We could relax this if we
> > > > > > added
> > > > > > a way of enforcing a power-of-two size. */
> > > > > > unsigned HOST_WIDE_INT size;
> > > > > > - if (!GET_MODE_SIZE (TYPE_MODE (vectype)).is_constant
> > > > > > (&size))
> > > > > > + if (!GET_MODE_SIZE (TYPE_MODE (vectype)).is_constant
> > > > > > ()
> > > > > > + || !DR_TARGET_ALIGNMENT (dr_info).is_constant
> > > > > > (&size))
> > > > >
> > > > > I agree that checking DR_TARGET_ALIGNMENT is what needs to be done,
> > > > > I'm not
> > > > sure
> > > > > why we checked the size - probably historic. But there's no need to
> > > > > keep the size
> > > > > check, so just
> > > > >
> > > > > diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> > > > > index b3ec0b67826..8d43b99a6f4 100644
> > > > > --- a/gcc/tree-vect-data-refs.cc
> > > > > +++ b/gcc/tree-vect-data-refs.cc
> > > > > @@ -2969,7 +2969,7 @@ vect_enhance_data_refs_alignment (loop_vec_info
> > > > > loop_vinfo)
> > > > > VF is a power of two. We could relax this if we
> > > > > added
> > > > > a way of enforcing a power-of-two size. */
> > > > > unsigned HOST_WIDE_INT size;
> > > > > - if (!GET_MODE_SIZE (TYPE_MODE (vectype)).is_constant
> > > > > (&size))
> > > > > + if (!DR_TARGET_ALIGNMENT (dr_info).is_constant (&size))
> > > > > {
> > > > > do_versioning = false;
> > > > > break;
> > > > >
> > > > > is correct. OK if that works, and sorry for the delay.
>
> Just noticed your proposal and Tamar's have a bit difference. I think Tamar's
> is right because we need to reject versioning for VLA.
>
> > target_alignment is what the target requires to make it aligned,
> > using the size is too pessimistic (and wrong in the early break case).
>
> The implementation in function vect_compute_data_ref_alignment of current GCC
> code is indeed like this. If dr_safe_speculative_read_required but
> new_alignment
> is not a constant poly, the vector_alignment can remain the element size. But
> I'm going to change this soon in my next patch which depends on this fix.
>
> > >
> > > The reason I still checked size is because if I'm not mistaken some VLA
> > > targets like SVE return that the desired alignment is the element size.
> > > So you'd get a non-poly constant there I believe, but we don't support
> > > peeling this was for VLA.
>
> > Yes, and using target_alignment will make it possible to use
> > alignment versioning in the first place (and SVE only requires
> > element alignment?)
>
> Can I check directly here whether the VF is a constant as below, which seems
> clearer.
>
> diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> index e7919b73c25..66217c54b05 100644
> --- a/gcc/tree-vect-data-refs.cc
> +++ b/gcc/tree-vect-data-refs.cc
> @@ -2969,7 +2969,8 @@ vect_enhance_data_refs_alignment (loop_vec_info
> loop_vinfo)
> VF is a power of two. We could relax this if we added
> a way of enforcing a power-of-two size. */
> unsigned HOST_WIDE_INT size;
> - if (!GET_MODE_SIZE (TYPE_MODE (vectype)).is_constant (&size))
> + if (!LOOP_VINFO_VECT_FACTOR (loop_vinfo).is_constant ()
> + || !DR_TARGET_ALIGNMENT (dr_info).is_constant (&size))
> {
> do_versioning = false;
> break;
But why do we need to reject VLA vectorization for versioning when
the target only requires element alignment? I could for example
think that arm could have gone with requiring NEON vector size
alignment for SVE accesses.
I do agree that keeping the old check is "safe", but I wonder why
this needs to reject a poly-int VF?
> --
> Thanks,
> Pengfei
>
>
>
>
--
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)
