On January 20, 2018 11:35:12 AM GMT+01:00, Richard Sandiford
<richard.sandif...@linaro.org> wrote:
>As Jakub says in the PR, the problem here was that the x86/built-in
>version of the scatter support was using a bogus scatter_src_dt
>when calling vect_get_vec_def_for_stmt_copy (and had since it
>was added). The patch uses the vect_def_type from the original
>call to vect_is_simple_use instead.
>
>However, Jakub also pointed out that other parts of the load and store
>code passed the vector operand rather than the scalar operand to
>vect_is_simple_use. That probably works most of the time since
>a constant scalar operand should give a constant vector operand,
>and likewise for external and internal definitions. But it
>definitely seems more robust to pass the scalar operand.
>
>The patch avoids the issue for gather and scatter offsets by
>using the cached gs_info.offset_dt. This is safe because gathers
>and scatters are never grouped, so there's only one statement operand
>to consider. The patch also caches the vect_def_type for mask
>operands,
>which is safe because grouped masked operations share the same mask.
>
>That just leaves the store rhs. We still need to recalculate the
>vect_def_type there since different store values in the group can
>have different definition types. But since we still have access
>to the original scalar operand, it seems better to use that instead.
>
>Tested on aarch64-linux-gnu, x86_64-linux-gnu and
>powerpc64le-linux-gnu.
>OK to install?
OK.
Thanks,
Richard.
>Richard
>
>
>2018-01-20 Richard Sandiford <richard.sandif...@linaro.org>
>
>gcc/
> PR tree-optimization/83940
> * tree-vect-stmts.c (vect_truncate_gather_scatter_offset): Set
> offset_dt to vect_constant_def rather than vect_unknown_def_type.
> (vect_check_load_store_mask): Add a mask_dt_out parameter and
> use it to pass back the definition type.
> (vect_check_store_rhs): Likewise rhs_dt_out.
> (vect_build_gather_load_calls): Add a mask_dt argument and use
> it instead of a call to vect_is_simple_use.
> (vectorizable_store): Update calls to vect_check_load_store_mask
> and vect_check_store_rhs. Use the dt returned by the latter instead
> of scatter_src_dt. Use the cached mask_dt and gs_info.offset_dt
> instead of calls to vect_is_simple_use. Pass the scalar rather
> than the vector operand to vect_is_simple_use when handling
> second and subsequent copies of an rhs value.
> (vectorizable_load): Update calls to vect_check_load_store_mask
> and vect_build_gather_load_calls. Use the cached mask_dt and
> gs_info.offset_dt instead of calls to vect_is_simple_use.
>
>gcc/testsuite/
> PR tree-optimization/83940
> * gcc.dg/torture/pr83940.c: New test.
>
>Index: gcc/tree-vect-stmts.c
>===================================================================
>--- gcc/tree-vect-stmts.c 2018-01-16 15:13:19.658832080 +0000
>+++ gcc/tree-vect-stmts.c 2018-01-20 10:31:16.692599735 +0000
>@@ -1932,7 +1932,7 @@ vect_truncate_gather_scatter_offset (gim
> but we don't need to store that here. */
> gs_info->base = NULL_TREE;
> gs_info->offset = fold_convert (offset_type, step);
>- gs_info->offset_dt = vect_unknown_def_type;
>+ gs_info->offset_dt = vect_constant_def;
> gs_info->offset_vectype = NULL_TREE;
> gs_info->scale = scale;
> gs_info->memory_type = memory_type;
>@@ -2374,11 +2374,14 @@ get_load_store_type (gimple *stmt, tree
> }
>
> /* Return true if boolean argument MASK is suitable for vectorizing
>- conditional load or store STMT. When returning true, store the
>- type of the vectorized mask in *MASK_VECTYPE_OUT. */
>+ conditional load or store STMT. When returning true, store the
>type
>+ of the definition in *MASK_DT_OUT and the type of the vectorized
>mask
>+ in *MASK_VECTYPE_OUT. */
>
> static bool
>-vect_check_load_store_mask (gimple *stmt, tree mask, tree
>*mask_vectype_out)
>+vect_check_load_store_mask (gimple *stmt, tree mask,
>+ vect_def_type *mask_dt_out,
>+ tree *mask_vectype_out)
> {
> if (!VECT_SCALAR_BOOLEAN_TYPE_P (TREE_TYPE (mask)))
> {
>@@ -2398,9 +2401,9 @@ vect_check_load_store_mask (gimple *stmt
>
> stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> gimple *def_stmt;
>- enum vect_def_type dt;
>+ enum vect_def_type mask_dt;
> tree mask_vectype;
>- if (!vect_is_simple_use (mask, stmt_info->vinfo, &def_stmt, &dt,
>+ if (!vect_is_simple_use (mask, stmt_info->vinfo, &def_stmt,
>&mask_dt,
> &mask_vectype))
> {
> if (dump_enabled_p ())
>@@ -2437,18 +2440,19 @@ vect_check_load_store_mask (gimple *stmt
> return false;
> }
>
>+ *mask_dt_out = mask_dt;
> *mask_vectype_out = mask_vectype;
> return true;
> }
>
> /* Return true if stored value RHS is suitable for vectorizing store
> statement STMT. When returning true, store the type of the
>- vectorized store value in *RHS_VECTYPE_OUT and the type of the
>- store in *VLS_TYPE_OUT. */
>+ definition in *RHS_DT_OUT, the type of the vectorized store value
>in
>+ *RHS_VECTYPE_OUT and the type of the store in *VLS_TYPE_OUT. */
>
> static bool
>-vect_check_store_rhs (gimple *stmt, tree rhs, tree *rhs_vectype_out,
>- vec_load_store_type *vls_type_out)
>+vect_check_store_rhs (gimple *stmt, tree rhs, vect_def_type
>*rhs_dt_out,
>+ tree *rhs_vectype_out, vec_load_store_type *vls_type_out)
> {
> /* In the case this is a store from a constant make sure
> native_encode_expr can handle it. */
>@@ -2462,9 +2466,9 @@ vect_check_store_rhs (gimple *stmt, tree
>
> stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> gimple *def_stmt;
>- enum vect_def_type dt;
>+ enum vect_def_type rhs_dt;
> tree rhs_vectype;
>- if (!vect_is_simple_use (rhs, stmt_info->vinfo, &def_stmt, &dt,
>+ if (!vect_is_simple_use (rhs, stmt_info->vinfo, &def_stmt, &rhs_dt,
> &rhs_vectype))
> {
> if (dump_enabled_p ())
>@@ -2482,8 +2486,9 @@ vect_check_store_rhs (gimple *stmt, tree
> return false;
> }
>
>+ *rhs_dt_out = rhs_dt;
> *rhs_vectype_out = rhs_vectype;
>- if (dt == vect_constant_def || dt == vect_external_def)
>+ if (rhs_dt == vect_constant_def || rhs_dt == vect_external_def)
> *vls_type_out = VLS_STORE_INVARIANT;
> else
> *vls_type_out = VLS_STORE;
>@@ -2546,12 +2551,12 @@ vect_build_zero_merge_argument (gimple *
>/* Build a gather load call while vectorizing STMT. Insert new
>instructions
>before GSI and add them to VEC_STMT. GS_INFO describes the gather load
> operation. If the load is conditional, MASK is the unvectorized
>- condition, otherwise MASK is null. */
>+ condition and MASK_DT is its definition type, otherwise MASK is
>null. */
>
> static void
> vect_build_gather_load_calls (gimple *stmt, gimple_stmt_iterator *gsi,
> gimple **vec_stmt, gather_scatter_info *gs_info,
>- tree mask)
>+ tree mask, vect_def_type mask_dt)
> {
> stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
>@@ -2682,12 +2687,7 @@ vect_build_gather_load_calls (gimple *st
> if (j == 0)
> vec_mask = vect_get_vec_def_for_operand (mask, stmt);
> else
>- {
>- gimple *def_stmt;
>- enum vect_def_type dt;
>- vect_is_simple_use (vec_mask, loop_vinfo, &def_stmt, &dt);
>- vec_mask = vect_get_vec_def_for_stmt_copy (dt, vec_mask);
>- }
>+ vec_mask = vect_get_vec_def_for_stmt_copy (mask_dt, vec_mask);
>
> mask_op = vec_mask;
> if (!useless_type_conversion_p (masktype, TREE_TYPE (vec_mask)))
>@@ -6057,7 +6057,8 @@ vectorizable_store (gimple *stmt, gimple
> tree dummy;
> enum dr_alignment_support alignment_support_scheme;
> gimple *def_stmt;
>- enum vect_def_type dt;
>+ enum vect_def_type rhs_dt = vect_unknown_def_type;
>+ enum vect_def_type mask_dt = vect_unknown_def_type;
> stmt_vec_info prev_stmt_info = NULL;
> tree dataref_ptr = NULL_TREE;
> tree dataref_offset = NULL_TREE;
>@@ -6078,7 +6079,6 @@ vectorizable_store (gimple *stmt, gimple
> vec_info *vinfo = stmt_info->vinfo;
> tree aggr_type;
> gather_scatter_info gs_info;
>- enum vect_def_type scatter_src_dt = vect_unknown_def_type;
> gimple *new_stmt;
> poly_uint64 vf;
> vec_load_store_type vls_type;
>@@ -6131,7 +6131,8 @@ vectorizable_store (gimple *stmt, gimple
> if (mask_index >= 0)
> {
> mask = gimple_call_arg (call, mask_index);
>- if (!vect_check_load_store_mask (stmt, mask, &mask_vectype))
>+ if (!vect_check_load_store_mask (stmt, mask, &mask_dt,
>+ &mask_vectype))
> return false;
> }
> }
>@@ -6172,7 +6173,7 @@ vectorizable_store (gimple *stmt, gimple
> return false;
> }
>
>- if (!vect_check_store_rhs (stmt, op, &rhs_vectype, &vls_type))
>+ if (!vect_check_store_rhs (stmt, op, &rhs_dt, &rhs_vectype,
>&vls_type))
> return false;
>
> elem_type = TREE_TYPE (vectype);
>@@ -6339,7 +6340,7 @@ vectorizable_store (gimple *stmt, gimple
> if (modifier == WIDEN)
> {
> src = vec_oprnd1
>- = vect_get_vec_def_for_stmt_copy (scatter_src_dt,
>vec_oprnd1);
>+ = vect_get_vec_def_for_stmt_copy (rhs_dt, vec_oprnd1);
> op = permute_vec_elements (vec_oprnd0, vec_oprnd0, perm_mask,
> stmt, gsi);
> }
>@@ -6357,7 +6358,7 @@ vectorizable_store (gimple *stmt, gimple
> else
> {
> src = vec_oprnd1
>- = vect_get_vec_def_for_stmt_copy (scatter_src_dt, vec_oprnd1);
>+ = vect_get_vec_def_for_stmt_copy (rhs_dt, vec_oprnd1);
> op = vec_oprnd0
> = vect_get_vec_def_for_stmt_copy (gs_info.offset_dt,
> vec_oprnd0);
>@@ -6616,8 +6617,9 @@ vectorizable_store (gimple *stmt, gimple
> vec_oprnd = vec_oprnds[j];
> else
> {
>- vect_is_simple_use (vec_oprnd, vinfo, &def_stmt, &dt);
>- vec_oprnd = vect_get_vec_def_for_stmt_copy (dt,
>vec_oprnd);
>+ vect_is_simple_use (op, vinfo, &def_stmt, &rhs_dt);
>+ vec_oprnd = vect_get_vec_def_for_stmt_copy (rhs_dt,
>+ vec_oprnd);
> }
> }
> /* Pun the vector to extract from if necessary. */
>@@ -6858,26 +6860,19 @@ vectorizable_store (gimple *stmt, gimple
> for (i = 0; i < group_size; i++)
> {
> op = oprnds[i];
>- vect_is_simple_use (op, vinfo, &def_stmt, &dt);
>- vec_oprnd = vect_get_vec_def_for_stmt_copy (dt, op);
>+ vect_is_simple_use (op, vinfo, &def_stmt, &rhs_dt);
>+ vec_oprnd = vect_get_vec_def_for_stmt_copy (rhs_dt, op);
> dr_chain[i] = vec_oprnd;
> oprnds[i] = vec_oprnd;
> }
> if (mask)
>- {
>- vect_is_simple_use (vec_mask, vinfo, &def_stmt, &dt);
>- vec_mask = vect_get_vec_def_for_stmt_copy (dt, vec_mask);
>- }
>+ vec_mask = vect_get_vec_def_for_stmt_copy (mask_dt, vec_mask);
> if (dataref_offset)
> dataref_offset
> = int_const_binop (PLUS_EXPR, dataref_offset, bump);
> else if (STMT_VINFO_GATHER_SCATTER_P (stmt_info))
>- {
>- gimple *def_stmt;
>- vect_def_type dt;
>- vect_is_simple_use (vec_offset, loop_vinfo, &def_stmt, &dt);
>- vec_offset = vect_get_vec_def_for_stmt_copy (dt, vec_offset);
>- }
>+ vec_offset = vect_get_vec_def_for_stmt_copy (gs_info.offset_dt,
>+ vec_offset);
> else
> dataref_ptr = bump_vector_ptr (dataref_ptr, ptr_incr, gsi, stmt,
> bump);
>@@ -7238,6 +7233,7 @@ vectorizable_load (gimple *stmt, gimple_
> gather_scatter_info gs_info;
> vec_info *vinfo = stmt_info->vinfo;
> tree ref_type;
>+ enum vect_def_type mask_dt = vect_unknown_def_type;
>
> if (!STMT_VINFO_RELEVANT_P (stmt_info) && !bb_vinfo)
> return false;
>@@ -7290,7 +7286,8 @@ vectorizable_load (gimple *stmt, gimple_
> if (mask_index >= 0)
> {
> mask = gimple_call_arg (call, mask_index);
>- if (!vect_check_load_store_mask (stmt, mask, &mask_vectype))
>+ if (!vect_check_load_store_mask (stmt, mask, &mask_dt,
>+ &mask_vectype))
> return false;
> }
> }
>@@ -7472,7 +7469,8 @@ vectorizable_load (gimple *stmt, gimple_
>
> if (memory_access_type == VMAT_GATHER_SCATTER && gs_info.decl)
> {
>- vect_build_gather_load_calls (stmt, gsi, vec_stmt, &gs_info,
>mask);
>+ vect_build_gather_load_calls (stmt, gsi, vec_stmt, &gs_info,
>mask,
>+ mask_dt);
> return true;
> }
>
>@@ -7999,22 +7997,13 @@ vectorizable_load (gimple *stmt, gimple_
> dataref_offset = int_const_binop (PLUS_EXPR, dataref_offset,
> bump);
> else if (STMT_VINFO_GATHER_SCATTER_P (stmt_info))
>- {
>- gimple *def_stmt;
>- vect_def_type dt;
>- vect_is_simple_use (vec_offset, loop_vinfo, &def_stmt, &dt);
>- vec_offset = vect_get_vec_def_for_stmt_copy (dt, vec_offset);
>- }
>+ vec_offset = vect_get_vec_def_for_stmt_copy (gs_info.offset_dt,
>+ vec_offset);
> else
> dataref_ptr = bump_vector_ptr (dataref_ptr, ptr_incr, gsi,
> stmt, bump);
> if (mask)
>- {
>- gimple *def_stmt;
>- vect_def_type dt;
>- vect_is_simple_use (vec_mask, vinfo, &def_stmt, &dt);
>- vec_mask = vect_get_vec_def_for_stmt_copy (dt, vec_mask);
>- }
>+ vec_mask = vect_get_vec_def_for_stmt_copy (mask_dt, vec_mask);
> }
>
> if (grouped_load || slp_perm)
>Index: gcc/testsuite/gcc.dg/torture/pr83940.c
>===================================================================
>--- /dev/null 2018-01-20 10:07:57.564678031 +0000
>+++ gcc/testsuite/gcc.dg/torture/pr83940.c 2018-01-20
>10:31:16.690598830 +0000
>@@ -0,0 +1,9 @@
>+/* { dg-do compile } */
>+/* { dg-options "-mavx512f" { target { i?86-*-* x86_64-*-* } } } */
>+
>+void
>+foo (double *a[], int b)
>+{
>+ for (; b; b++)
>+ a[b][b] = 1.0;
>+}
