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.
--
Thanks,
Pengfei
