> -----Original Message-----
> From: Pengfei Li <pengfei....@arm.com>
> Sent: Tuesday, July 29, 2025 10:17 AM
> To: Richard Biener <richard.guent...@gmail.com>; Tamar Christina
> <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; rguent...@suse.de; s...@gentoo.org
> Subject: Re: [PATCH] vect: Fix insufficient alignment requirement for
> speculative
> loads [PR121190]
>
> Hi,
>
> Thanks Tamar and Richi for the review.
>
> > > I wonder about what the intention of this code was. It seems to me that
> > > it was
> > > trying to disable versioning for VLA, but then also doubling up and using
> > > the
> > > mode as the alignment. But the cross iteration alignment check below
> > > this on
> > > uses DR_TARGET_ALIGNMENT as one would expect, since the target alignment
> > > can be different from the vector size.
> > >
> > > So I wonder if something like this isn't more appropriate:
> > >
> > > 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.
>
> I had compared the approach I used in the patch with the one Tamar suggested.
> Both have their pros and cons. I chose the one in my patch because I think it
> avoids unnecessarily increasing alignment requirements if no speculative load
> exists.
>
> But I would adopt your suggestion if you both think it would be better
> overall.
> I will update this and the test case, and post a new version soon.
>
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.
Regards,
Tamar
> --
> Thanks,
> Pengfei
