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