]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
tree-optimization/96920 - another ICE when vectorizing nested cycles
authorRichard Biener <rguenther@suse.de>
Fri, 4 Sep 2020 12:35:39 +0000 (14:35 +0200)
committerRichard Biener <rguenther@suse.de>
Wed, 2 Dec 2020 10:06:42 +0000 (11:06 +0100)
This refines the previous fix for PR96698 by re-doing how and where
we arrange for setting vectorized cycle PHI backedge values.

2020-09-04  Richard Biener  <rguenther@suse.de>

PR tree-optimization/96698
PR tree-optimization/96920
* tree-vectorizer.h (loop_vec_info::reduc_latch_defs): Remove.
(loop_vec_info::reduc_latch_slp_defs): Likewise.
* tree-vect-stmts.c (vect_transform_stmt): Remove vectorized
cycle PHI latch code.
* tree-vect-loop.c (maybe_set_vectorized_backedge_value): New
helper to set vectorized cycle PHI latch values.
(vect_transform_loop): Walk over all PHIs again after
vectorizing them, calling maybe_set_vectorized_backedge_value.
Call maybe_set_vectorized_backedge_value for each vectorized
stmt.  Remove delayed update code.
* tree-vect-slp.c (vect_analyze_slp_instance): Initialize
SLP instance reduc_phis member.
(vect_schedule_slp): Set vectorized cycle PHI latch values.

* gfortran.dg/vect/pr96920.f90: New testcase.
* gcc.dg/vect/pr96920.c: Likewise.

(cherry picked from commit 46a58c779af3055a4b10b285a1f4be28abe4351c)

gcc/testsuite/gcc.dg/vect/pr96920.c [new file with mode: 0644]
gcc/testsuite/gfortran.dg/vect/pr96920.f90 [new file with mode: 0644]
gcc/tree-vect-loop.c
gcc/tree-vect-slp.c
gcc/tree-vect-stmts.c
gcc/tree-vectorizer.h

diff --git a/gcc/testsuite/gcc.dg/vect/pr96920.c b/gcc/testsuite/gcc.dg/vect/pr96920.c
new file mode 100644 (file)
index 0000000..af5da4a
--- /dev/null
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+
+int a[1024];
+int b[2048];
+
+void foo (int x, int y)
+{
+  for (int i = 0; i < 1024; ++i)
+    {
+      int tem0 = b[2*i];
+      int tem1 = b[2*i+1];
+      for (int j = 0; j < 32; ++j)
+       {
+         int tem = tem0;
+         tem0 = tem1;
+         tem1 = tem;
+         a[i] += tem0;
+       }
+    }
+}
diff --git a/gcc/testsuite/gfortran.dg/vect/pr96920.f90 b/gcc/testsuite/gfortran.dg/vect/pr96920.f90
new file mode 100644 (file)
index 0000000..e1dc0ad
--- /dev/null
@@ -0,0 +1,37 @@
+! { dg-do compile }
+      subroutine ice(npoint, nterm, x, g)
+      implicit none
+      integer    norder
+      parameter (norder=10)
+      integer j
+      integer k
+      integer ii
+      integer nterm
+      integer npoint
+      real b(norder)
+      real c(norder)
+      real d(norder)
+      real x(npoint)
+      real g(npoint)
+      real gg
+      real prev
+      real prev2
+
+          j = 1
+    100   continue
+          j = j+1
+          if (nterm == j)  then
+             do ii=1,npoint
+                k = nterm
+                gg= d(k)
+                prev= 0.0
+                do k=k-1,1,-1
+                   prev2= prev
+                   prev= gg
+                   gg = d(k)+(x(ii)-b(k))*prev-c(k+1)*prev2
+                enddo
+                g(ii) = gg
+             enddo
+          endif
+          go to 100
+      end
index c84a502aa5af0b28c80ee84640df65cdd03344fe..3f6a28642b961c001cbcb77bc8f8dfa6dc8748c0 100644 (file)
@@ -8338,11 +8338,52 @@ scale_profile_for_vect_loop (class loop *loop, unsigned vf)
     scale_bbs_frequencies (&loop->latch, 1, exit_l->probability / prob);
 }
 
+/* For a vectorized stmt DEF_STMT_INFO adjust all vectorized PHI
+   latch edge values originally defined by it.  */
+
+static void
+maybe_set_vectorized_backedge_value (loop_vec_info loop_vinfo,
+                                    stmt_vec_info def_stmt_info)
+{
+  tree def = gimple_get_lhs (vect_orig_stmt (def_stmt_info)->stmt);
+  if (!def || TREE_CODE (def) != SSA_NAME)
+    return;
+  stmt_vec_info phi_info;
+  imm_use_iterator iter;
+  use_operand_p use_p;
+  FOR_EACH_IMM_USE_FAST (use_p, iter, def)
+    if (gphi *phi = dyn_cast <gphi *> (USE_STMT (use_p)))
+      if (gimple_bb (phi)->loop_father->header == gimple_bb (phi)
+         && (phi_info = loop_vinfo->lookup_stmt (phi))
+         && VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (phi_info))
+         && STMT_VINFO_REDUC_TYPE (phi_info) != FOLD_LEFT_REDUCTION
+         && STMT_VINFO_REDUC_TYPE (phi_info) != EXTRACT_LAST_REDUCTION)
+       {
+         loop_p loop = gimple_bb (phi)->loop_father;
+         edge e = loop_latch_edge (loop);
+         if (PHI_ARG_DEF_FROM_EDGE (phi, e) == def)
+           {
+             stmt_vec_info phi_vec_info = STMT_VINFO_VEC_STMT (phi_info);
+             stmt_vec_info def_vec_info = STMT_VINFO_VEC_STMT (def_stmt_info);
+             do
+               {
+                 add_phi_arg (as_a <gphi *> (phi_vec_info->stmt),
+                              gimple_get_lhs (def_vec_info->stmt), e,
+                              gimple_phi_arg_location (phi, e->dest_idx));
+                 phi_vec_info = STMT_VINFO_RELATED_STMT (phi_vec_info);
+                 def_vec_info = STMT_VINFO_RELATED_STMT (def_vec_info);
+               }
+             while (phi_vec_info);
+             gcc_assert (!def_vec_info);
+           }
+       }
+}
+
 /* Vectorize STMT_INFO if relevant, inserting any new instructions before GSI.
    When vectorizing STMT_INFO as a store, set *SEEN_STORE to its
    stmt_vec_info.  */
 
-static void
+static bool
 vect_transform_loop_stmt (loop_vec_info loop_vinfo, stmt_vec_info stmt_info,
                          gimple_stmt_iterator *gsi, stmt_vec_info *seen_store)
 {
@@ -8358,7 +8399,7 @@ vect_transform_loop_stmt (loop_vec_info loop_vinfo, stmt_vec_info stmt_info,
 
   if (!STMT_VINFO_RELEVANT_P (stmt_info)
       && !STMT_VINFO_LIVE_P (stmt_info))
-    return;
+    return false;
 
   if (STMT_VINFO_VECTYPE (stmt_info))
     {
@@ -8375,13 +8416,15 @@ vect_transform_loop_stmt (loop_vec_info loop_vinfo, stmt_vec_info stmt_info,
   /* Pure SLP statements have already been vectorized.  We still need
      to apply loop vectorization to hybrid SLP statements.  */
   if (PURE_SLP_STMT (stmt_info))
-    return;
+    return false;
 
   if (dump_enabled_p ())
     dump_printf_loc (MSG_NOTE, vect_location, "transform statement.\n");
 
   if (vect_transform_stmt (stmt_info, gsi, NULL, NULL))
     *seen_store = stmt_info;
+
+  return true;
 }
 
 /* Helper function to pass to simplify_replace_tree to enable replacing tree's
@@ -8708,7 +8751,7 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call)
 
       for (gphi_iterator si = gsi_start_phis (bb); !gsi_end_p (si);
           gsi_next (&si))
-        {
+       {
          gphi *phi = si.phi ();
          if (dump_enabled_p ())
            dump_printf_loc (MSG_NOTE, vect_location,
@@ -8743,6 +8786,27 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call)
            }
        }
 
+      for (gphi_iterator si = gsi_start_phis (bb); !gsi_end_p (si);
+          gsi_next (&si))
+       {
+         gphi *phi = si.phi ();
+         stmt_info = loop_vinfo->lookup_stmt (phi);
+         if (!stmt_info)
+           continue;
+
+         if (!STMT_VINFO_RELEVANT_P (stmt_info)
+             && !STMT_VINFO_LIVE_P (stmt_info))
+           continue;
+
+         if ((STMT_VINFO_DEF_TYPE (stmt_info) == vect_induction_def
+              || STMT_VINFO_DEF_TYPE (stmt_info) == vect_reduction_def
+              || STMT_VINFO_DEF_TYPE (stmt_info) == vect_double_reduction_def
+              || STMT_VINFO_DEF_TYPE (stmt_info) == vect_nested_cycle
+              || STMT_VINFO_DEF_TYPE (stmt_info) == vect_internal_def)
+             && ! PURE_SLP_STMT (stmt_info))
+           maybe_set_vectorized_backedge_value (loop_vinfo, stmt_info);
+       }
+
       for (gimple_stmt_iterator si = gsi_start_bb (bb);
           !gsi_end_p (si);)
        {
@@ -8777,11 +8841,18 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call)
                        }
                      stmt_vec_info pat_stmt_info
                        = STMT_VINFO_RELATED_STMT (stmt_info);
-                     vect_transform_loop_stmt (loop_vinfo, pat_stmt_info, &si,
-                                               &seen_store);
+                     if (vect_transform_loop_stmt (loop_vinfo, pat_stmt_info,
+                                                   &si, &seen_store))
+                       maybe_set_vectorized_backedge_value (loop_vinfo,
+                                                            pat_stmt_info);
+                   }
+                 else
+                   {
+                     if (vect_transform_loop_stmt (loop_vinfo, stmt_info, &si,
+                                                   &seen_store))
+                       maybe_set_vectorized_backedge_value (loop_vinfo,
+                                                            stmt_info);
                    }
-                 vect_transform_loop_stmt (loop_vinfo, stmt_info, &si,
-                                           &seen_store);
                }
              gsi_next (&si);
              if (seen_store)
@@ -8798,38 +8869,6 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call)
            }
        }
 
-      /* Fill in backedge defs of reductions.  */
-      for (unsigned i = 0; i < loop_vinfo->reduc_latch_defs.length (); ++i)
-       {
-         stmt_vec_info stmt_info = loop_vinfo->reduc_latch_defs[i];
-         stmt_vec_info orig_stmt_info = vect_orig_stmt (stmt_info);
-         vec<gimple *> &phi_info
-           = STMT_VINFO_VEC_STMTS (STMT_VINFO_REDUC_DEF (orig_stmt_info));
-         vec<gimple *> &vec_stmt
-           = STMT_VINFO_VEC_STMTS (stmt_info);
-         gcc_assert (phi_info.length () == vec_stmt.length ());
-         gphi *phi
-           = dyn_cast <gphi *> (STMT_VINFO_REDUC_DEF (orig_stmt_info)->stmt);
-         edge e = loop_latch_edge (gimple_bb (phi_info[0])->loop_father);
-         for (unsigned j = 0; j < phi_info.length (); ++j)
-           add_phi_arg (as_a <gphi *> (phi_info[j]),
-                        gimple_get_lhs (vec_stmt[j]), e,
-                        gimple_phi_arg_location (phi, e->dest_idx));
-       }
-      for (unsigned i = 0; i < loop_vinfo->reduc_latch_slp_defs.length (); ++i)
-       {
-         slp_tree slp_node = loop_vinfo->reduc_latch_slp_defs[i].first;
-         slp_tree phi_node = loop_vinfo->reduc_latch_slp_defs[i].second;
-         gphi *phi = as_a <gphi *> (SLP_TREE_SCALAR_STMTS (phi_node)[0]->stmt);
-         e = loop_latch_edge (gimple_bb (phi)->loop_father);
-         gcc_assert (SLP_TREE_VEC_STMTS (phi_node).length ()
-                     == SLP_TREE_VEC_STMTS (slp_node).length ());
-         for (unsigned j = 0; j < SLP_TREE_VEC_STMTS (phi_node).length (); ++j)
-           add_phi_arg (as_a <gphi *> (SLP_TREE_VEC_STMTS (phi_node)[j]),
-                        vect_get_slp_vect_def (slp_node, j),
-                        e, gimple_phi_arg_location (phi, e->dest_idx));
-       }
-
       /* Stub out scalar statements that must not survive vectorization.
         Doing this here helps with grouped statements, or statements that
         are involved in patterns.  */
index 3a8bbeaf6579edfee9c0f9e6f2c723ac5bc4a39f..70d2bb20a835b9f336d151ae44f654c8c41fbf80 100644 (file)
@@ -2281,6 +2281,7 @@ vect_analyze_slp_instance (vec_info *vinfo,
          SLP_INSTANCE_UNROLLING_FACTOR (new_instance) = unrolling_factor;
          SLP_INSTANCE_LOADS (new_instance) = vNULL;
          SLP_INSTANCE_ROOT_STMT (new_instance) = constructor ? stmt_info : NULL;
+         new_instance->reduc_phis = NULL;
 
          vect_gather_slp_loads (new_instance, node);
          if (dump_enabled_p ())
@@ -4408,6 +4409,25 @@ vect_schedule_slp (vec_info *vinfo)
       stmt_vec_info store_info;
       unsigned int j;
 
+      /* For reductions set the latch values of the vectorized PHIs.  */
+      if (instance->reduc_phis
+         && STMT_VINFO_REDUC_TYPE (SLP_TREE_SCALAR_STMTS
+                       (instance->reduc_phis)[0]) != FOLD_LEFT_REDUCTION
+         && STMT_VINFO_REDUC_TYPE (SLP_TREE_SCALAR_STMTS
+                       (instance->reduc_phis)[0]) != EXTRACT_LAST_REDUCTION)
+       {
+         slp_tree slp_node = root;
+         slp_tree phi_node = instance->reduc_phis;
+         gphi *phi = as_a <gphi *> (SLP_TREE_SCALAR_STMTS (phi_node)[0]->stmt);
+         edge e = loop_latch_edge (gimple_bb (phi)->loop_father);
+         gcc_assert (SLP_TREE_VEC_STMTS (phi_node).length ()
+                     == SLP_TREE_VEC_STMTS (slp_node).length ());
+         for (unsigned j = 0; j < SLP_TREE_VEC_STMTS (phi_node).length (); ++j)
+           add_phi_arg (as_a <gphi *> (SLP_TREE_VEC_STMTS (phi_node)[j]->stmt),
+                        gimple_get_lhs (SLP_TREE_VEC_STMTS (slp_node)[j]->stmt),
+                        e, gimple_phi_arg_location (phi, e->dest_idx));
+       }
+
       /* Remove scalar call stmts.  Do not do this for basic-block
         vectorization as not all uses may be vectorized.
         ???  Why should this be necessary?  DCE should be able to
index edf28924dceec132094aa6ac57eac1db128077f0..65b258d249dba9bedf69c06f183bed855fd679ec 100644 (file)
@@ -11190,33 +11190,6 @@ vect_transform_stmt (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
   if (STMT_VINFO_TYPE (stmt_info) == store_vec_info_type)
     return is_store;
 
-  /* If this stmt defines a value used on a backedge, record it so
-     we can update the vectorized PHIs later.  */
-  stmt_vec_info orig_stmt_info = vect_orig_stmt (stmt_info);
-  stmt_vec_info reduc_info;
-  if (STMT_VINFO_REDUC_DEF (orig_stmt_info)
-      && vect_stmt_to_vectorize (orig_stmt_info) == stmt_info
-      && (reduc_info = info_for_reduction (orig_stmt_info))
-      && STMT_VINFO_REDUC_TYPE (reduc_info) != FOLD_LEFT_REDUCTION
-      && STMT_VINFO_REDUC_TYPE (reduc_info) != EXTRACT_LAST_REDUCTION)
-    {
-      gphi *phi;
-      edge e;
-      if (!slp_node
-         && (phi = dyn_cast <gphi *>
-                     (STMT_VINFO_REDUC_DEF (orig_stmt_info)->stmt))
-         && dominated_by_p (CDI_DOMINATORS,
-                            gimple_bb (orig_stmt_info->stmt), gimple_bb (phi))
-         && (e = loop_latch_edge (gimple_bb (phi)->loop_father))
-         && (PHI_ARG_DEF_FROM_EDGE (phi, e)
-             == gimple_get_lhs (orig_stmt_info->stmt)))
-       as_a <loop_vec_info> (vinfo)->reduc_latch_defs.safe_push (stmt_info);
-      else if (slp_node
-              && slp_node != slp_node_instance->reduc_phis)
-       as_a <loop_vec_info> (vinfo)->reduc_latch_slp_defs.safe_push
-           (std::make_pair (slp_node, slp_node_instance->reduc_phis));
-    }
-
   /* Handle stmts whose DEF is used outside the loop-nest that is
      being vectorized.  */
   done = can_vectorize_live_stmts (stmt_info, gsi, slp_node,
index 69aa0fff40461f43e05890c9a64f9d7d25c84ccb..f7becb34ab41c645e5e76065377d78f2af39a09a 100644 (file)
@@ -586,11 +586,6 @@ public:
      stmt in the chain.  */
   auto_vec<stmt_vec_info> reduction_chains;
 
-  /* The vectorized stmts defining the latch values of the reduction
-     they are involved with.  */
-  auto_vec<stmt_vec_info> reduc_latch_defs;
-  auto_vec<std::pair<slp_tree, slp_tree> > reduc_latch_slp_defs;
-
   /* Cost vector for a single scalar iteration.  */
   auto_vec<stmt_info_for_cost> scalar_cost_vec;