Fix vect_def_type handling in x86 scatter support (PR 83940)

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.

2018-01-20  Richard Sandiford  <richard.sandiford@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.

From-SVN: r256918

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 321066c..f41e1ad 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,23 @@
+2018-01-20  Richard Sandiford  <richard.sandiford@linaro.org>
+
+       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.
+
 2018-01-20  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/83945

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a88e459..7b48adb 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-20  Richard Sandiford  <richard.sandiford@linaro.org>
+
+       PR tree-optimization/83940
+       * gcc.dg/torture/pr83940.c: New test.
+
 2018-01-20  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/83945

diff --git a/gcc/testsuite/gcc.dg/torture/pr83940.c b/gcc/testsuite/gcc.dg/torture/pr83940.c
new file mode 100644 (file)
index 0000000..49a00fd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr83940.c
@@ -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;
+}

diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 3f8eb92..da76572 100644 (file)
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -1932,7 +1932,7 @@ vect_truncate_gather_scatter_offset (gimple *stmt, loop_vec_info loop_vinfo,
         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 vectype, bool slp, bool masked_p,
 }
 
 /* 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, tree mask, tree *mask_vectype_out)
 
   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, tree mask, tree *mask_vectype_out)
       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 rhs, tree *rhs_vectype_out,
 
   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 rhs, tree *rhs_vectype_out,
       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 *stmt, tree vectype)
 /* 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 *stmt, gimple_stmt_iterator *gsi,
              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_stmt_iterator *gsi, gimple **vec_stmt,
   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_stmt_iterator *gsi, gimple **vec_stmt,
   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_stmt_iterator *gsi, gimple **vec_stmt,
       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_stmt_iterator *gsi, gimple **vec_stmt,
       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_stmt_iterator *gsi, gimple **vec_stmt,
              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_stmt_iterator *gsi, gimple **vec_stmt,
          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_stmt_iterator *gsi, gimple **vec_stmt,
                    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_stmt_iterator *gsi, gimple **vec_stmt,
          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_stmt_iterator *gsi, gimple **vec_stmt,
   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_stmt_iterator *gsi, gimple **vec_stmt,
       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_stmt_iterator *gsi, gimple **vec_stmt,
 
   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_stmt_iterator *gsi, gimple **vec_stmt,
            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)