Re: [PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-15 Thread Richard Sandiford via Gcc-patches
Richard Biener via Gcc-patches  writes:
> On Wed, 15 Sep 2021, Richard Sandiford wrote:
>> Richard Biener  writes:
>> > On Tue, 14 Sep 2021, Richard Sandiford wrote:
>> >
>> >> Richard Biener via Gcc-patches  writes:
>> >> > This changes us to maintain and compute (mis-)alignment info for
>> >> > the first element of a group only rather than for each DR when
>> >> > doing interleaving and for the earliest, first, or first in the SLP
>> >> > node (or any pair or all three of those) when SLP vectorizing.
>> >> >
>> >> > For this to work out the easiest way I have changed the accessors
>> >> > DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
>> >> > the first element rather than adjusting all callers.
>> >> > dr_misalignment is moved out-of-line and I'm not too fond of the
>> >> > poly-int dances there (any hints?), but basically we are now
>> >> > adjusting the first elements misalignment based on the DR_INIT
>> >> > difference.
>> >> >
>> >> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>> >> >
>> >> > Richard.
>> >> >
>> >> > 2021-09-13  Richard Biener  
>> >> >
>> >> > * tree-vectorizer.h (dr_misalignment): Move out of line.
>> >> > (dr_target_alignment): New.
>> >> > (DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
>> >> > (set_dr_target_alignment): New.
>> >> > (SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
>> >> > * tree-vect-data-refs.c (dr_misalignment): Compute and
>> >> > return the group members misalignment.
>> >> > (vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
>> >> > (vect_analyze_data_refs_alignment): Compute alignment only
>> >> > for the first element of a DR group.
>> >> > (vect_slp_analyze_node_alignment): Likewise.
>> >> > ---
>> >> >  gcc/tree-vect-data-refs.c | 65 ---
>> >> >  gcc/tree-vectorizer.h | 24 ++-
>> >> >  2 files changed, 57 insertions(+), 32 deletions(-)
>> >> >
>> >> > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>> >> > index 66e76132d14..b53d6a0b3f1 100644
>> >> > --- a/gcc/tree-vect-data-refs.c
>> >> > +++ b/gcc/tree-vect-data-refs.c
>> >> > @@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info 
>> >> > *vinfo, slp_instance instance)
>> >> >return res;
>> >> >  }
>> >> >  
>> >> > +/* Return the misalignment of DR_INFO.  */
>> >> > +
>> >> > +int
>> >> > +dr_misalignment (dr_vec_info *dr_info)
>> >> > +{
>> >> > +  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
>> >> > +{
>> >> > +  dr_vec_info *first_dr
>> >> > +   = STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
>> >> > +  int misalign = first_dr->misalignment;
>> >> > +  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
>> >> > +  if (misalign == DR_MISALIGNMENT_UNKNOWN)
>> >> > +   return misalign;
>> >> > +  poly_offset_int diff = (wi::to_poly_offset (DR_INIT 
>> >> > (dr_info->dr))
>> >> > + - wi::to_poly_offset (DR_INIT 
>> >> > (first_dr->dr)));
>> >> > +  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
>> >> > +  bool res = known_misalignment (mispoly,
>> >> > +
>> >> > first_dr->target_alignment.to_constant (),
>> >> > +);
>> >> > +  gcc_assert (res);
>> >> > +  return misalign;
>> >> 
>> >> Yeah, not too keen on the to_constants here.  The one on diff looks
>> >> redundant -- you could just use diff.force_shwi () instead, and
>> >> keep everything poly_int.
>> >>
>> >> For the known_misalignment I think we should use:
>> >> 
>> >>if (!can_div_trunc_p (mispoly, first_dr->target_alignment,
>> >>, ))
>> >>  misalign = DR_MISALIGNMENT_UNKNOWN;
>> >>return misalign;
>> >> 
>> >> There are then no to_constant assumptions.
>> >
>> > OK, note that group analysis does
>> >
>> >   /* Check that the DR_INITs are compile-time constants.  */
>> >   if (TREE_CODE (DR_INIT (dra)) != INTEGER_CST
>> >   || TREE_CODE (DR_INIT (drb)) != INTEGER_CST)
>> > break;
>> >
>> >   /* Sorting has ensured that DR_INIT (dra) <= DR_INIT (drb).  */
>> >   HOST_WIDE_INT init_a = TREE_INT_CST_LOW (DR_INIT (dra));
>> >   HOST_WIDE_INT init_b = TREE_INT_CST_LOW (DR_INIT (drb));
>> >
>> > so I'm confident my variant was "correct", but it still was ugly.
>> 
>> Ah, OK.  In that case I don't mind the original version, but it would be
>> good to have a comment above the to_constant saying where the condition
>> is enforced.
>> 
>> I'm just trying to avoid to_constant calls with no comment to explain
>> them, and with no nearby is_constant call.  Otherwise it could end up
>> a bit like tree_to_uhwi, where sometimes tree_fits_uhwi_p really has
>> been checked earlier (not always obvious where) and sometimes
>> tree_to_uhwi is just used out of 

Re: [PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-15 Thread Richard Biener via Gcc-patches
On Wed, 15 Sep 2021, Richard Sandiford wrote:

> Richard Biener  writes:
> > On Tue, 14 Sep 2021, Richard Sandiford wrote:
> >
> >> Richard Biener via Gcc-patches  writes:
> >> > This changes us to maintain and compute (mis-)alignment info for
> >> > the first element of a group only rather than for each DR when
> >> > doing interleaving and for the earliest, first, or first in the SLP
> >> > node (or any pair or all three of those) when SLP vectorizing.
> >> >
> >> > For this to work out the easiest way I have changed the accessors
> >> > DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
> >> > the first element rather than adjusting all callers.
> >> > dr_misalignment is moved out-of-line and I'm not too fond of the
> >> > poly-int dances there (any hints?), but basically we are now
> >> > adjusting the first elements misalignment based on the DR_INIT
> >> > difference.
> >> >
> >> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >> >
> >> > Richard.
> >> >
> >> > 2021-09-13  Richard Biener  
> >> >
> >> >  * tree-vectorizer.h (dr_misalignment): Move out of line.
> >> >  (dr_target_alignment): New.
> >> >  (DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
> >> >  (set_dr_target_alignment): New.
> >> >  (SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
> >> >  * tree-vect-data-refs.c (dr_misalignment): Compute and
> >> >  return the group members misalignment.
> >> >  (vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
> >> >  (vect_analyze_data_refs_alignment): Compute alignment only
> >> >  for the first element of a DR group.
> >> >  (vect_slp_analyze_node_alignment): Likewise.
> >> > ---
> >> >  gcc/tree-vect-data-refs.c | 65 ---
> >> >  gcc/tree-vectorizer.h | 24 ++-
> >> >  2 files changed, 57 insertions(+), 32 deletions(-)
> >> >
> >> > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> >> > index 66e76132d14..b53d6a0b3f1 100644
> >> > --- a/gcc/tree-vect-data-refs.c
> >> > +++ b/gcc/tree-vect-data-refs.c
> >> > @@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info 
> >> > *vinfo, slp_instance instance)
> >> >return res;
> >> >  }
> >> >  
> >> > +/* Return the misalignment of DR_INFO.  */
> >> > +
> >> > +int
> >> > +dr_misalignment (dr_vec_info *dr_info)
> >> > +{
> >> > +  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
> >> > +{
> >> > +  dr_vec_info *first_dr
> >> > += STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
> >> > +  int misalign = first_dr->misalignment;
> >> > +  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
> >> > +  if (misalign == DR_MISALIGNMENT_UNKNOWN)
> >> > +return misalign;
> >> > +  poly_offset_int diff = (wi::to_poly_offset (DR_INIT (dr_info->dr))
> >> > +  - wi::to_poly_offset (DR_INIT 
> >> > (first_dr->dr)));
> >> > +  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
> >> > +  bool res = known_misalignment (mispoly,
> >> > + 
> >> > first_dr->target_alignment.to_constant (),
> >> > + );
> >> > +  gcc_assert (res);
> >> > +  return misalign;
> >> 
> >> Yeah, not too keen on the to_constants here.  The one on diff looks
> >> redundant -- you could just use diff.force_shwi () instead, and
> >> keep everything poly_int.
> >>
> >> For the known_misalignment I think we should use:
> >> 
> >>if (!can_div_trunc_p (mispoly, first_dr->target_alignment,
> >> , ))
> >>  misalign = DR_MISALIGNMENT_UNKNOWN;
> >>return misalign;
> >> 
> >> There are then no to_constant assumptions.
> >
> > OK, note that group analysis does
> >
> >   /* Check that the DR_INITs are compile-time constants.  */
> >   if (TREE_CODE (DR_INIT (dra)) != INTEGER_CST
> >   || TREE_CODE (DR_INIT (drb)) != INTEGER_CST)
> > break;
> >
> >   /* Sorting has ensured that DR_INIT (dra) <= DR_INIT (drb).  */
> >   HOST_WIDE_INT init_a = TREE_INT_CST_LOW (DR_INIT (dra));
> >   HOST_WIDE_INT init_b = TREE_INT_CST_LOW (DR_INIT (drb));
> >
> > so I'm confident my variant was "correct", but it still was ugly.
> 
> Ah, OK.  In that case I don't mind the original version, but it would be
> good to have a comment above the to_constant saying where the condition
> is enforced.
> 
> I'm just trying to avoid to_constant calls with no comment to explain
> them, and with no nearby is_constant call.  Otherwise it could end up
> a bit like tree_to_uhwi, where sometimes tree_fits_uhwi_p really has
> been checked earlier (not always obvious where) and sometimes
> tree_to_uhwi is just used out of hope, to avoid having to think about
> the alternative.
> 
> > There's also the issue that target_alignment is poly_uint64 but
> > misalignment is signed int.
> >
> > Note that can_div_trunc_p seems to require a poly_uint64 

Re: [PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-15 Thread Richard Sandiford via Gcc-patches
Richard Biener  writes:
> On Tue, 14 Sep 2021, Richard Sandiford wrote:
>
>> Richard Biener via Gcc-patches  writes:
>> > This changes us to maintain and compute (mis-)alignment info for
>> > the first element of a group only rather than for each DR when
>> > doing interleaving and for the earliest, first, or first in the SLP
>> > node (or any pair or all three of those) when SLP vectorizing.
>> >
>> > For this to work out the easiest way I have changed the accessors
>> > DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
>> > the first element rather than adjusting all callers.
>> > dr_misalignment is moved out-of-line and I'm not too fond of the
>> > poly-int dances there (any hints?), but basically we are now
>> > adjusting the first elements misalignment based on the DR_INIT
>> > difference.
>> >
>> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>> >
>> > Richard.
>> >
>> > 2021-09-13  Richard Biener  
>> >
>> >* tree-vectorizer.h (dr_misalignment): Move out of line.
>> >(dr_target_alignment): New.
>> >(DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
>> >(set_dr_target_alignment): New.
>> >(SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
>> >* tree-vect-data-refs.c (dr_misalignment): Compute and
>> >return the group members misalignment.
>> >(vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
>> >(vect_analyze_data_refs_alignment): Compute alignment only
>> >for the first element of a DR group.
>> >(vect_slp_analyze_node_alignment): Likewise.
>> > ---
>> >  gcc/tree-vect-data-refs.c | 65 ---
>> >  gcc/tree-vectorizer.h | 24 ++-
>> >  2 files changed, 57 insertions(+), 32 deletions(-)
>> >
>> > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>> > index 66e76132d14..b53d6a0b3f1 100644
>> > --- a/gcc/tree-vect-data-refs.c
>> > +++ b/gcc/tree-vect-data-refs.c
>> > @@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info 
>> > *vinfo, slp_instance instance)
>> >return res;
>> >  }
>> >  
>> > +/* Return the misalignment of DR_INFO.  */
>> > +
>> > +int
>> > +dr_misalignment (dr_vec_info *dr_info)
>> > +{
>> > +  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
>> > +{
>> > +  dr_vec_info *first_dr
>> > +  = STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
>> > +  int misalign = first_dr->misalignment;
>> > +  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
>> > +  if (misalign == DR_MISALIGNMENT_UNKNOWN)
>> > +  return misalign;
>> > +  poly_offset_int diff = (wi::to_poly_offset (DR_INIT (dr_info->dr))
>> > +- wi::to_poly_offset (DR_INIT (first_dr->dr)));
>> > +  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
>> > +  bool res = known_misalignment (mispoly,
>> > +   first_dr->target_alignment.to_constant (),
>> > +   );
>> > +  gcc_assert (res);
>> > +  return misalign;
>> 
>> Yeah, not too keen on the to_constants here.  The one on diff looks
>> redundant -- you could just use diff.force_shwi () instead, and
>> keep everything poly_int.
>>
>> For the known_misalignment I think we should use:
>> 
>>if (!can_div_trunc_p (mispoly, first_dr->target_alignment,
>>   , ))
>>  misalign = DR_MISALIGNMENT_UNKNOWN;
>>return misalign;
>> 
>> There are then no to_constant assumptions.
>
> OK, note that group analysis does
>
>   /* Check that the DR_INITs are compile-time constants.  */
>   if (TREE_CODE (DR_INIT (dra)) != INTEGER_CST
>   || TREE_CODE (DR_INIT (drb)) != INTEGER_CST)
> break;
>
>   /* Sorting has ensured that DR_INIT (dra) <= DR_INIT (drb).  */
>   HOST_WIDE_INT init_a = TREE_INT_CST_LOW (DR_INIT (dra));
>   HOST_WIDE_INT init_b = TREE_INT_CST_LOW (DR_INIT (drb));
>
> so I'm confident my variant was "correct", but it still was ugly.

Ah, OK.  In that case I don't mind the original version, but it would be
good to have a comment above the to_constant saying where the condition
is enforced.

I'm just trying to avoid to_constant calls with no comment to explain
them, and with no nearby is_constant call.  Otherwise it could end up
a bit like tree_to_uhwi, where sometimes tree_fits_uhwi_p really has
been checked earlier (not always obvious where) and sometimes
tree_to_uhwi is just used out of hope, to avoid having to think about
the alternative.

> There's also the issue that target_alignment is poly_uint64 but
> misalignment is signed int.
>
> Note that can_div_trunc_p seems to require a poly_uint64 remainder,
> I'm not sure what to do with that, so I used is_constant.

Ah, yeah, forgot about that sorry.  I guess in that case, using
is_constant on first_dr->target_alignment and sticking with
known_misalignment would make sense.

> Btw, to what value do we want to align with variable sized 

Re: [PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-15 Thread Richard Biener via Gcc-patches
On Tue, 14 Sep 2021, Richard Sandiford wrote:

> Richard Biener via Gcc-patches  writes:
> > This changes us to maintain and compute (mis-)alignment info for
> > the first element of a group only rather than for each DR when
> > doing interleaving and for the earliest, first, or first in the SLP
> > node (or any pair or all three of those) when SLP vectorizing.
> >
> > For this to work out the easiest way I have changed the accessors
> > DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
> > the first element rather than adjusting all callers.
> > dr_misalignment is moved out-of-line and I'm not too fond of the
> > poly-int dances there (any hints?), but basically we are now
> > adjusting the first elements misalignment based on the DR_INIT
> > difference.
> >
> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >
> > Richard.
> >
> > 2021-09-13  Richard Biener  
> >
> > * tree-vectorizer.h (dr_misalignment): Move out of line.
> > (dr_target_alignment): New.
> > (DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
> > (set_dr_target_alignment): New.
> > (SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
> > * tree-vect-data-refs.c (dr_misalignment): Compute and
> > return the group members misalignment.
> > (vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
> > (vect_analyze_data_refs_alignment): Compute alignment only
> > for the first element of a DR group.
> > (vect_slp_analyze_node_alignment): Likewise.
> > ---
> >  gcc/tree-vect-data-refs.c | 65 ---
> >  gcc/tree-vectorizer.h | 24 ++-
> >  2 files changed, 57 insertions(+), 32 deletions(-)
> >
> > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> > index 66e76132d14..b53d6a0b3f1 100644
> > --- a/gcc/tree-vect-data-refs.c
> > +++ b/gcc/tree-vect-data-refs.c
> > @@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info *vinfo, 
> > slp_instance instance)
> >return res;
> >  }
> >  
> > +/* Return the misalignment of DR_INFO.  */
> > +
> > +int
> > +dr_misalignment (dr_vec_info *dr_info)
> > +{
> > +  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
> > +{
> > +  dr_vec_info *first_dr
> > +   = STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
> > +  int misalign = first_dr->misalignment;
> > +  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
> > +  if (misalign == DR_MISALIGNMENT_UNKNOWN)
> > +   return misalign;
> > +  poly_offset_int diff = (wi::to_poly_offset (DR_INIT (dr_info->dr))
> > + - wi::to_poly_offset (DR_INIT (first_dr->dr)));
> > +  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
> > +  bool res = known_misalignment (mispoly,
> > +first_dr->target_alignment.to_constant (),
> > +);
> > +  gcc_assert (res);
> > +  return misalign;
> 
> Yeah, not too keen on the to_constants here.  The one on diff looks
> redundant -- you could just use diff.force_shwi () instead, and
> keep everything poly_int.
>
> For the known_misalignment I think we should use:
> 
>if (!can_div_trunc_p (mispoly, first_dr->target_alignment,
>, ))
>  misalign = DR_MISALIGNMENT_UNKNOWN;
>return misalign;
> 
> There are then no to_constant assumptions.

OK, note that group analysis does

  /* Check that the DR_INITs are compile-time constants.  */
  if (TREE_CODE (DR_INIT (dra)) != INTEGER_CST
  || TREE_CODE (DR_INIT (drb)) != INTEGER_CST)
break;

  /* Sorting has ensured that DR_INIT (dra) <= DR_INIT (drb).  */
  HOST_WIDE_INT init_a = TREE_INT_CST_LOW (DR_INIT (dra));
  HOST_WIDE_INT init_b = TREE_INT_CST_LOW (DR_INIT (drb));

so I'm confident my variant was "correct", but it still was ugly.
There's also the issue that target_alignment is poly_uint64 but
misalignment is signed int.

Note that can_div_trunc_p seems to require a poly_uint64 remainder,
I'm not sure what to do with that, so I used is_constant.

Btw, to what value do we want to align with variable sized vectors?
We are aligning globals according to target_alignment but only
support that for constant alignment.  Most users only want to
distinguish between aligned or not aligned and the actual misalignment
is only used to seed the SSA alignment info, so I suppose being
too conservative in dr_misalignment for variable size vectors isn't
too bad if we correctly identify fully aligned accesses?

I'm now testing a variant that looks like

int
dr_misalignment (dr_vec_info *dr_info)
{
  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
{
  dr_vec_info *first_dr
= STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
  int misalign = first_dr->misalignment;
  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
  if (misalign == DR_MISALIGNMENT_UNKNOWN)

Re: [PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-14 Thread Richard Sandiford via Gcc-patches
Richard Biener via Gcc-patches  writes:
> This changes us to maintain and compute (mis-)alignment info for
> the first element of a group only rather than for each DR when
> doing interleaving and for the earliest, first, or first in the SLP
> node (or any pair or all three of those) when SLP vectorizing.
>
> For this to work out the easiest way I have changed the accessors
> DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
> the first element rather than adjusting all callers.
> dr_misalignment is moved out-of-line and I'm not too fond of the
> poly-int dances there (any hints?), but basically we are now
> adjusting the first elements misalignment based on the DR_INIT
> difference.
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2021-09-13  Richard Biener  
>
>   * tree-vectorizer.h (dr_misalignment): Move out of line.
>   (dr_target_alignment): New.
>   (DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
>   (set_dr_target_alignment): New.
>   (SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
>   * tree-vect-data-refs.c (dr_misalignment): Compute and
>   return the group members misalignment.
>   (vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
>   (vect_analyze_data_refs_alignment): Compute alignment only
>   for the first element of a DR group.
>   (vect_slp_analyze_node_alignment): Likewise.
> ---
>  gcc/tree-vect-data-refs.c | 65 ---
>  gcc/tree-vectorizer.h | 24 ++-
>  2 files changed, 57 insertions(+), 32 deletions(-)
>
> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
> index 66e76132d14..b53d6a0b3f1 100644
> --- a/gcc/tree-vect-data-refs.c
> +++ b/gcc/tree-vect-data-refs.c
> @@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info *vinfo, 
> slp_instance instance)
>return res;
>  }
>  
> +/* Return the misalignment of DR_INFO.  */
> +
> +int
> +dr_misalignment (dr_vec_info *dr_info)
> +{
> +  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
> +{
> +  dr_vec_info *first_dr
> + = STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
> +  int misalign = first_dr->misalignment;
> +  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
> +  if (misalign == DR_MISALIGNMENT_UNKNOWN)
> + return misalign;
> +  poly_offset_int diff = (wi::to_poly_offset (DR_INIT (dr_info->dr))
> +   - wi::to_poly_offset (DR_INIT (first_dr->dr)));
> +  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
> +  bool res = known_misalignment (mispoly,
> +  first_dr->target_alignment.to_constant (),
> +  );
> +  gcc_assert (res);
> +  return misalign;

Yeah, not too keen on the to_constants here.  The one on diff looks
redundant -- you could just use diff.force_shwi () instead, and
keep everything poly_int.

For the known_misalignment I think we should use:

   if (!can_div_trunc_p (mispoly, first_dr->target_alignment,
 , ))
 misalign = DR_MISALIGNMENT_UNKNOWN;
   return misalign;

There are then no to_constant assumptions.

Thanks,
Richard


[PATCH] Maintain (mis-)alignment info in the first element of a group

2021-09-13 Thread Richard Biener via Gcc-patches
This changes us to maintain and compute (mis-)alignment info for
the first element of a group only rather than for each DR when
doing interleaving and for the earliest, first, or first in the SLP
node (or any pair or all three of those) when SLP vectorizing.

For this to work out the easiest way I have changed the accessors
DR_MISALIGNMENT and DR_TARGET_ALIGNMENT to do the indirection to
the first element rather than adjusting all callers.
dr_misalignment is moved out-of-line and I'm not too fond of the
poly-int dances there (any hints?), but basically we are now
adjusting the first elements misalignment based on the DR_INIT
difference.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2021-09-13  Richard Biener  

* tree-vectorizer.h (dr_misalignment): Move out of line.
(dr_target_alignment): New.
(DR_TARGET_ALIGNMENT): Wrap dr_target_alignment.
(set_dr_target_alignment): New.
(SET_DR_TARGET_ALIGNMENT): Wrap set_dr_target_alignment.
* tree-vect-data-refs.c (dr_misalignment): Compute and
return the group members misalignment.
(vect_compute_data_ref_alignment): Use SET_DR_TARGET_ALIGNMENT.
(vect_analyze_data_refs_alignment): Compute alignment only
for the first element of a DR group.
(vect_slp_analyze_node_alignment): Likewise.
---
 gcc/tree-vect-data-refs.c | 65 ---
 gcc/tree-vectorizer.h | 24 ++-
 2 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 66e76132d14..b53d6a0b3f1 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -887,6 +887,36 @@ vect_slp_analyze_instance_dependence (vec_info *vinfo, 
slp_instance instance)
   return res;
 }
 
+/* Return the misalignment of DR_INFO.  */
+
+int
+dr_misalignment (dr_vec_info *dr_info)
+{
+  if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt))
+{
+  dr_vec_info *first_dr
+   = STMT_VINFO_DR_INFO (DR_GROUP_FIRST_ELEMENT (dr_info->stmt));
+  int misalign = first_dr->misalignment;
+  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
+  if (misalign == DR_MISALIGNMENT_UNKNOWN)
+   return misalign;
+  poly_offset_int diff = (wi::to_poly_offset (DR_INIT (dr_info->dr))
+ - wi::to_poly_offset (DR_INIT (first_dr->dr)));
+  poly_int64 mispoly = misalign + diff.to_constant ().to_shwi ();
+  bool res = known_misalignment (mispoly,
+first_dr->target_alignment.to_constant (),
+);
+  gcc_assert (res);
+  return misalign;
+}
+  else
+{
+  int misalign = dr_info->misalignment;
+  gcc_assert (misalign != DR_MISALIGNMENT_UNINITIALIZED);
+  return misalign;
+}
+}
+
 /* Record the base alignment guarantee given by DRB, which occurs
in STMT_INFO.  */
 
@@ -992,7 +1022,7 @@ vect_compute_data_ref_alignment (vec_info *vinfo, 
dr_vec_info *dr_info)
 
   poly_uint64 vector_alignment
 = exact_div (vect_calculate_target_alignment (dr_info), BITS_PER_UNIT);
-  DR_TARGET_ALIGNMENT (dr_info) = vector_alignment;
+  SET_DR_TARGET_ALIGNMENT (dr_info, vector_alignment);
 
   /* If the main loop has peeled for alignment we have no way of knowing
  whether the data accesses in the epilogues are aligned.  We can't at
@@ -2408,7 +2438,12 @@ vect_analyze_data_refs_alignment (loop_vec_info 
loop_vinfo)
 {
   dr_vec_info *dr_info = loop_vinfo->lookup_dr (dr);
   if (STMT_VINFO_VECTORIZABLE (dr_info->stmt))
-   vect_compute_data_ref_alignment (loop_vinfo, dr_info);
+   {
+ if (STMT_VINFO_GROUPED_ACCESS (dr_info->stmt)
+ && DR_GROUP_FIRST_ELEMENT (dr_info->stmt) != dr_info->stmt)
+   continue;
+ vect_compute_data_ref_alignment (loop_vinfo, dr_info);
+   }
 }
 
   return opt_result::success ();
@@ -2420,13 +2455,9 @@ vect_analyze_data_refs_alignment (loop_vec_info 
loop_vinfo)
 static bool
 vect_slp_analyze_node_alignment (vec_info *vinfo, slp_tree node)
 {
-  /* We vectorize from the first scalar stmt in the node unless
- the node is permuted in which case we start from the first
- element in the group.  */
+  /* Alignment is maintained in the first element of the group.  */
   stmt_vec_info first_stmt_info = SLP_TREE_SCALAR_STMTS (node)[0];
-  dr_vec_info *first_dr_info = STMT_VINFO_DR_INFO (first_stmt_info);
-  if (SLP_TREE_LOAD_PERMUTATION (node).exists ())
-first_stmt_info = DR_GROUP_FIRST_ELEMENT (first_stmt_info);
+  first_stmt_info = DR_GROUP_FIRST_ELEMENT (first_stmt_info);
 
   /* We need to commit to a vector type for the group now.  */
   if (is_a  (vinfo)
@@ -2440,22 +2471,8 @@ vect_slp_analyze_node_alignment (vec_info *vinfo, 
slp_tree node)
 }
 
   dr_vec_info *dr_info = STMT_VINFO_DR_INFO (first_stmt_info);
-  vect_compute_data_ref_alignment (vinfo, dr_info);
-  /* In several places