[2/3][vect] Add widening add, subtract vect patterns

[Joel Hutton]

Tests are still running, but I believe I've addressed all the comments.

> Like Richard said, the new patterns need to be documented in md.texi
> and the new tree codes need to be documented in generic.texi.

Done.

> While we're using tree codes, I think we need to make the naming
> consistent with other tree codes: WIDEN_PLUS_EXPR instead of
> WIDEN_ADD_EXPR and WIDEN_MINUS_EXPR instead of WIDEN_SUB_EXPR.
> Same idea for the VEC_* codes.

Fixed.

> > gcc/ChangeLog:
> >
> > 2020-11-12  Joel Hutton  <joel.hutton@arm.com>
> >
> >         * expr.c (expand_expr_real_2): add widen_add,widen_subtract cases
> 
> Not that I personally care about this stuff (would love to see changelogs
> go away :-)) but some nits:
> 
> Each description is supposed to start with a capital letter and end with
> a full stop (even if it's not a complete sentence).  Same for the rest

Fixed.

> >         * optabs-tree.c (optab_for_tree_code): optabs for widening adds,subtracts
> 
> The line limit for changelogs is 80 characters.  The entry should say
> what changed, so “Handle …” or “Add case for …” or something.

Fixed.

> >         * tree-vect-patterns.c (vect_recog_widen_add_pattern): New recog ptatern
> 
> typo: pattern

Fixed.

> > Add widening add, subtract patterns to tree-vect-patterns.
> > Add aarch64 tests for patterns.
> >
> > fix sad
> 
> Would be good to expand on this for the final commit message.

'fix sad' was accidentally included when I squashed two commits. I've made all the commit messages more descriptive.

> > +
> > +    case VEC_WIDEN_SUB_HI_EXPR:
> > +      return (TYPE_UNSIGNED (type)
> > +           ? vec_widen_usubl_hi_optab  : vec_widen_ssubl_hi_optab);
> > +
> > +
> 
> Nits: excess blank line at the end and excess space before the “:”s.

Fixed.

> > +OPTAB_D (vec_widen_usubl_lo_optab, "vec_widen_usubl_lo_$a")
> > +OPTAB_D (vec_widen_uaddl_hi_optab, "vec_widen_uaddl_hi_$a")
> > +OPTAB_D (vec_widen_uaddl_lo_optab, "vec_widen_uaddl_lo_$a")
> >  OPTAB_D (vec_widen_sshiftl_hi_optab, "vec_widen_sshiftl_hi_$a")
> >  OPTAB_D (vec_widen_sshiftl_lo_optab, "vec_widen_sshiftl_lo_$a")
> >  OPTAB_D (vec_widen_umult_even_optab, "vec_widen_umult_even_$a")
> 
> Looks like the current code groups signed stuff together and
> unsigned stuff together, so would be good to follow that.

Fixed.

> Same comments as the previous patch about having a "+nosve" pragma
> and about the scan-assembler-times lines.  Same for the sub test.

Fixed.

> I am missing documentation in md.texi for the new patterns.  In
> particular I wonder why you need singed and unsigned variants
> for the add/subtract patterns.

Fixed. Signed and unsigned variants because they correspond to signed and
unsigned instructions, (uaddl/uaddl2, saddl/saddl2).

> The new functions should have comments before them.  Can probably
> just use the vect_recog_widen_mult_pattern comment as a template.

Fixed.

> > +    case VEC_WIDEN_SUB_HI_EXPR:
> > +    case VEC_WIDEN_SUB_LO_EXPR:
> > +    case VEC_WIDEN_ADD_HI_EXPR:
> > +    case VEC_WIDEN_ADD_LO_EXPR:
> > +      return false;
> > +
>
> I think these should get the same validity checking as
> VEC_WIDEN_MULT_HI_EXPR etc.

Fixed.

> > --- a/gcc/tree-vect-patterns.c
> > +++ b/gcc/tree-vect-patterns.c
> > @@ -1086,8 +1086,10 @@ vect_recog_sad_pattern (vec_info *vinfo,
> >       of the above pattern.  */
> >
> >    tree plus_oprnd0, plus_oprnd1;
> > -  if (!vect_reassociating_reduction_p (vinfo, stmt_vinfo, PLUS_EXPR,
> > -                                    &plus_oprnd0, &plus_oprnd1))
> > +  if (!(vect_reassociating_reduction_p (vinfo, stmt_vinfo, PLUS_EXPR,
> > +                                    &plus_oprnd0, &plus_oprnd1)
> > +     || vect_reassociating_reduction_p (vinfo, stmt_vinfo, WIDEN_ADD_EXPR,
> > +                                    &plus_oprnd0, &plus_oprnd1)))
> >      return NULL;
> >
> >     tree sum_type = gimple_expr_type (last_stmt);
>
> I think we should make:
>
>   /* Any non-truncating sequence of conversions is OK here, since
>      with a successful match, the result of the ABS(U) is known to fit
>      within the nonnegative range of the result type.  (It cannot be the
>      negative of the minimum signed value due to the range of the widening
>      MINUS_EXPR.)  */
>   vect_unpromoted_value unprom_abs;
>   plus_oprnd0 = vect_look_through_possible_promotion (vinfo, plus_oprnd0,
>                                                       &unprom_abs);
>
> specific to the PLUS_EXPR case.  If we look through promotions on
> the operands of a WIDEN_ADD_EXPR, we could potentially have a mixture
> of signednesses involved, one on the operands of the WIDEN_ADD_EXPR
> and one on its inputs.

Fixed.


gcc/ChangeLog:

2020-11-13  Joel Hutton  <joel.hutton@arm.com>

	* expr.c (expand_expr_real_2): Add widen_add,widen_subtract cases.
	* optabs-tree.c (optab_for_tree_code): Add case for widening optabs.
	  adds, subtracts.
        * optabs.def (OPTAB_D): Define vectorized widen add, subtracts.
	* tree-cfg.c (verify_gimple_assign_binary): Add case for widening adds,
	  subtracts.
	* tree-inline.c (estimate_operator_cost): Add case for widening adds,
	   subtracts.
	* tree-vect-generic.c (expand_vector_operations_1): Add case for
	  widening adds, subtracts tree-vect-patterns.c
	* (vect_recog_widen_add_pattern): New recog pattern.
	  (vect_recog_widen_sub_pattern): New recog pattern.
	  (vect_recog_average_pattern): Update widened add code.
	  (vect_recog_average_pattern): Update widened add code.
	* tree-vect-stmts.c (vectorizable_conversion): Add case for widened add,
	  subtract.
	(supportable_widening_operation): Add case for widened add, subtract.
	* tree.def
	(WIDEN_PLUS_EXPR): New tree code.
	(WIDEN_MINUS_EXPR): New tree code.
	(VEC_WIDEN_ADD_HI_EXPR): New tree code.
	(VEC_WIDEN_PLUS_LO_EXPR): New tree code.
	(VEC_WIDEN_MINUS_HI_EXPR): New tree code.
	(VEC_WIDEN_MINUS_LO_EXPR): New tree code.

gcc/testsuite/ChangeLog:

2020-11-13  Joel Hutton  <joel.hutton@arm.com>

        * gcc.target/aarch64/vect-widen-add.c: New test.
        * gcc.target/aarch64/vect-widen-sub.c: New test.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-vect-Add-widening-add-subtract-patterns.patch
Type: text/x-patch
Size: 21630 bytes
Desc: 0002-vect-Add-widening-add-subtract-patterns.patch
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20201113/2b59f9a7/attachment-0001.bin>