This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PING] RE: [PATCH] Vectorization for store with negative step
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bingfeng Mei <bmei at broadcom dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 20 Dec 2013 14:23:59 +0100
- Subject: Re: [PING] RE: [PATCH] Vectorization for store with negative step
- Authentication-results: sourceware.org; auth=none
- References: <B71DF1153024A14EABB94E39368E44A604261295 at SJEXCHMB13 dot corp dot ad dot broadcom dot com>
On Fri, Dec 20, 2013 at 11:09 AM, Bingfeng Mei <bmei@broadcom.com> wrote:
> OK to commit?
Ok.
Thanks,
Richard.
> Thanks,
> Bingfeng
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Bingfeng Mei
> Sent: 18 December 2013 16:25
> To: Jakub Jelinek
> Cc: Richard Biener; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Vectorization for store with negative step
>
> Hi, Jakub,
> Sorry for all the formatting issues. Haven't submit a patch for a while :-).
> Please find the updated patch.
>
> Thanks,
> Bingfeng
>
> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: 18 December 2013 13:38
> To: Bingfeng Mei
> Cc: Richard Biener; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Vectorization for store with negative step
>
> On Wed, Dec 18, 2013 at 01:31:05PM +0000, Bingfeng Mei wrote:
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog (revision 206016)
> +++ gcc/ChangeLog (working copy)
> @@ -1,3 +1,9 @@
> +2013-12-18 Bingfeng Mei <bmei@broadcom.com>
> +
> + PR tree-optimization/59544
> + * tree-vect-stmts.c (perm_mask_for_reverse): Move before
>
> This should be a tab instead of 8 spaces.
>
> + vectorizable_store. (vectorizable_store): Handle negative step.
>
> Newline and tab after "store.", rather than space.
>
> Property changes on: gcc/testsuite/gcc.target/i386/pr59544.c
> ___________________________________________________________________
> Added: svn:executable
> + *
>
> Please don't add such bogus property. Testcases aren't executable.
>
> Index: gcc/testsuite/ChangeLog
> ===================================================================
> --- gcc/testsuite/ChangeLog (revision 206016)
> +++ gcc/testsuite/ChangeLog (working copy)
> @@ -1,3 +1,8 @@
> +2013-12-18 Bingfeng Mei <bmei@broadcom.com>
> +
> + PR tree-optimization/59544
> + * gcc.target/i386/pr59544.c: New test
>
> Missing dot at the end of line.
> +
> 2013-12-16 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/58956
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c (revision 206016)
> +++ gcc/tree-vect-stmts.c (working copy)
> @@ -4859,6 +4859,25 @@ ensure_base_align (stmt_vec_info stmt_in
> }
>
>
> +/* Given a vector type VECTYPE returns the VECTOR_CST mask that implements
> + reversal of the vector elements. If that is impossible to do,
> + returns NULL. */
> +
> +static tree
> +perm_mask_for_reverse (tree vectype)
> +{
> + int i, nunits;
> + unsigned char *sel;
> +
> + nunits = TYPE_VECTOR_SUBPARTS (vectype);
> + sel = XALLOCAVEC (unsigned char, nunits);
> +
> + for (i = 0; i < nunits; ++i)
> + sel[i] = nunits - 1 - i;
> +
> + return vect_gen_perm_mask (vectype, sel);
> +}
> +
> /* Function vectorizable_store.
>
> Check if STMT defines a non scalar data-ref (array/pointer/structure) that
> @@ -4902,6 +4921,8 @@ vectorizable_store (gimple stmt, gimple_
> vec<tree> oprnds = vNULL;
> vec<tree> result_chain = vNULL;
> bool inv_p;
> + bool negative = false;
> + tree offset = NULL_TREE;
> vec<tree> vec_oprnds = vNULL;
> bool slp = (slp_node != NULL);
> unsigned int vec_num;
> @@ -4976,16 +4997,38 @@ vectorizable_store (gimple stmt, gimple_
> if (!STMT_VINFO_DATA_REF (stmt_info))
> return false;
>
> - if (tree_int_cst_compare (loop && nested_in_vect_loop_p (loop, stmt)
> - ? STMT_VINFO_DR_STEP (stmt_info) : DR_STEP (dr),
> - size_zero_node) < 0)
> + negative = tree_int_cst_compare (loop && nested_in_vect_loop_p (loop, stmt)
> + ? STMT_VINFO_DR_STEP (stmt_info) : DR_STEP (dr),
> + size_zero_node) < 0;
>
> The formatting looks wrong, do:
> negative
> = tree_int_cst_compare (loop && nested_in_vect_loop_p (loop, stmt)
> ? STMT_VINFO_DR_STEP (stmt_info) : DR_STEP (dr),
> size_zero_node) < 0;
> instead.
>
> + if (negative && ncopies > 1)
> {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> - "negative step for store.\n");
> + "multiple types with negative step.");
> return false;
> }
>
> + if (negative)
> + {
> + gcc_assert (!grouped_store);
> + alignment_support_scheme = vect_supportable_dr_alignment (dr, false);
> + if (alignment_support_scheme != dr_aligned
> + && alignment_support_scheme != dr_unaligned_supported)
>
> Lots of places where you use 8 spaces instead of tab, please fix.
> + offset = size_int (-TYPE_VECTOR_SUBPARTS (vectype) + 1);
> +
> if (store_lanes_p)
> aggr_type = build_array_type_nelts (elem_type, vec_num * nunits);
> else
> @@ -5200,7 +5246,7 @@ vectorizable_store (gimple stmt, gimple_
> dataref_ptr
> = vect_create_data_ref_ptr (first_stmt, aggr_type,
> simd_lane_access_p ? loop : NULL,
> - NULL_TREE, &dummy, gsi, &ptr_incr,
> + offset, &dummy, gsi, &ptr_incr,
> simd_lane_access_p, &inv_p);
> gcc_assert (bb_vinfo || !inv_p);
> }
> @@ -5306,6 +5352,21 @@ vectorizable_store (gimple stmt, gimple_
> set_ptr_info_alignment (get_ptr_info (dataref_ptr), align,
> misalign);
>
> + if (negative)
> + {
> + tree perm_mask = perm_mask_for_reverse (vectype);
> + tree perm_dest = vect_create_destination_var (gimple_assign_rhs1 (stmt), vectype);
> + tree new_temp = make_ssa_name (perm_dest, NULL);
> +
> + /* Generate the permute statement. */
> + gimple perm_stmt = gimple_build_assign_with_ops (VEC_PERM_EXPR, new_temp,
> + vec_oprnd, vec_oprnd, perm_mask);
>
> Too long lines.
>
> Jakub