]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
middle-end: Handle resized PHI nodes in loop_version()
authorLewis Hyatt <lhyatt@gmail.com>
Sat, 7 Dec 2024 00:01:27 +0000 (19:01 -0500)
committerLewis Hyatt <lhyatt@gcc.gnu.org>
Sat, 7 Dec 2024 00:01:28 +0000 (19:01 -0500)
While testing upcoming support for 64-bit location_t, I came across some
test failures on sparc (32-bit) that trigger when location_t is changed to
be 64-bit. The reason is that several call sites that make use of
loop_version() for performing loop optimizations assume that a gphi*
obtained prior to calling loop_version() will remain valid afterwards, but
this is not the case for a PHI that needs to be resized. It doesn't happen
usually, because PHI nodes usually have room for at least 4 arguments and
this is usually more than are needed, but this is not guaranteed.

Fix the affected callers by avoiding the assumption that a PHI node pointer
remains valid. For most cases, this is done by remembering instead the
gphi->result pointer, which contains a pointer back to the PHI node that is
kept up to date when the PHI is moved to a new address.

gcc/ChangeLog:

* tree-parloops.cc (struct reduction_info): Store the result of the
reduction PHI rather than the PHI itself.
(reduction_info::reduc_phi): New member function.
(reduction_hasher::equal): Adapt to the change in struct reduction_info.
(reduction_phi): Likewise.
(initialize_reductions): Likewise.
(create_call_for_reduction_1): Likewise.
(transform_to_exit_first_loop_alt): Likewise.
(transform_to_exit_first_loop): Likewise.
(build_new_reduction): Likewise.
(set_reduc_phi_uids): Likewise.
(try_create_reduction_list): Likewise.
* tree-ssa-loop-split.cc (split_loop): Remember the PHI result
variable so that the PHI can be found in case it is resized and move
to a new address.
* tree-vect-loop-manip.cc (vect_loop_versioning): After calling
loop_version(), fix up stored PHI pointers in case they have
changed.
* tree-vectorizer.cc (vec_info::resync_stmt_addr): New function.
* tree-vectorizer.h (vec_info::resync_stmt_addr): Declare.

gcc/tree-parloops.cc
gcc/tree-ssa-loop-split.cc
gcc/tree-vect-loop-manip.cc
gcc/tree-vectorizer.cc
gcc/tree-vectorizer.h

index 13d8e84bc8f3a1d9d6936257acc9f385fb3b69ff..8427c287a6a770077fa06dabb73e8b002adc1629 100644 (file)
@@ -895,7 +895,7 @@ parloops_force_simple_reduction (loop_vec_info loop_info, stmt_vec_info phi_info
 struct reduction_info
 {
   gimple *reduc_stmt;          /* reduction statement.  */
-  gimple *reduc_phi;           /* The phi node defining the reduction.  */
+  tree reduc_phi_name;         /* The result of the phi node defining the reduction.  */
   enum tree_code reduction_code;/* code for the reduction operation.  */
   unsigned reduc_version;      /* SSA_NAME_VERSION of original reduc_phi
                                   result.  */
@@ -910,6 +910,12 @@ struct reduction_info
                                   will be passed to the atomic operation.  Represents
                                   the local result each thread computed for the reduction
                                   operation.  */
+
+  gphi *
+  reduc_phi () const
+  {
+    return as_a<gphi *> (SSA_NAME_DEF_STMT (reduc_phi_name));
+  }
 };
 
 /* Reduction info hashtable helpers.  */
@@ -925,7 +931,7 @@ struct reduction_hasher : free_ptr_hash <reduction_info>
 inline bool
 reduction_hasher::equal (const reduction_info *a, const reduction_info *b)
 {
-  return (a->reduc_phi == b->reduc_phi);
+  return (a->reduc_phi_name == b->reduc_phi_name);
 }
 
 inline hashval_t
@@ -949,10 +955,10 @@ reduction_phi (reduction_info_table_type *reduction_list, gimple *phi)
       || gimple_uid (phi) == 0)
     return NULL;
 
-  tmpred.reduc_phi = phi;
+  tmpred.reduc_phi_name = gimple_phi_result (phi);
   tmpred.reduc_version = gimple_uid (phi);
   red = reduction_list->find (&tmpred);
-  gcc_assert (red == NULL || red->reduc_phi == phi);
+  gcc_assert (red == NULL || red->reduc_phi () == phi);
 
   return red;
 }
@@ -1294,7 +1300,7 @@ initialize_reductions (reduction_info **slot, class loop *loop)
      from the preheader with the reduction initialization value.  */
 
   /* Initialize the reduction.  */
-  type = TREE_TYPE (PHI_RESULT (reduc->reduc_phi));
+  type = TREE_TYPE (reduc->reduc_phi_name);
   init = omp_reduction_init_op (gimple_location (reduc->reduc_stmt),
                                reduc->reduction_code, type);
   reduc->init = init;
@@ -1308,11 +1314,11 @@ initialize_reductions (reduction_info **slot, class loop *loop)
      computing is done.  */
 
   e = loop_preheader_edge (loop);
-  arg = PHI_ARG_DEF_FROM_EDGE (reduc->reduc_phi, e);
+  const auto phi = reduc->reduc_phi ();
+  arg = PHI_ARG_DEF_FROM_EDGE (phi, e);
   /* Create new variable to hold the initial value.  */
 
-  SET_USE (PHI_ARG_DEF_PTR_FROM_EDGE
-          (reduc->reduc_phi, loop_preheader_edge (loop)), init);
+  SET_USE (PHI_ARG_DEF_PTR_FROM_EDGE (phi, loop_preheader_edge (loop)), init);
   reduc->initial_value = arg;
   return 1;
 }
@@ -1795,7 +1801,7 @@ create_call_for_reduction_1 (reduction_info **slot, struct clsn_data *clsn_data)
 {
   struct reduction_info *const reduc = *slot;
   gimple_stmt_iterator gsi;
-  tree type = TREE_TYPE (PHI_RESULT (reduc->reduc_phi));
+  tree type = TREE_TYPE (reduc->reduc_phi_name);
   tree load_struct;
   basic_block bb;
   basic_block new_bb;
@@ -2424,8 +2430,9 @@ transform_to_exit_first_loop_alt (class loop *loop,
       if (red)
        {
          /* Register the new reduction phi.  */
-         red->reduc_phi = nphi;
-         gimple_set_uid (red->reduc_phi, red->reduc_version);
+         red->reduc_phi_name = res_c;
+         gcc_checking_assert (red->reduc_phi () == nphi);
+         gimple_set_uid (nphi, red->reduc_version);
        }
     }
   gcc_assert (gsi_end_p (gsi) && !v->iterate (i, &vm));
@@ -2651,6 +2658,8 @@ transform_to_exit_first_loop (class loop *loop,
       phi = gsi.phi ();
       res = PHI_RESULT (phi);
       t = copy_ssa_name (res, phi);
+      if (auto red = reduction_phi (reduction_list, phi))
+       red->reduc_phi_name = t;
       SET_PHI_RESULT (phi, t);
       nphi = create_phi_node (res, orig_header);
       add_phi_arg (nphi, t, hpred, UNKNOWN_LOCATION);
@@ -3263,8 +3272,9 @@ build_new_reduction (reduction_info_table_type *reduction_list,
   new_reduction = XCNEW (struct reduction_info);
 
   new_reduction->reduc_stmt = reduc_stmt;
-  new_reduction->reduc_phi = phi;
-  new_reduction->reduc_version = SSA_NAME_VERSION (gimple_phi_result (phi));
+  const auto phi_name = gimple_phi_result (phi);
+  new_reduction->reduc_phi_name = phi_name;
+  new_reduction->reduc_version = SSA_NAME_VERSION (phi_name);
   new_reduction->reduction_code = reduction_code;
   slot = reduction_list->find_slot (new_reduction, INSERT);
   *slot = new_reduction;
@@ -3276,7 +3286,7 @@ int
 set_reduc_phi_uids (reduction_info **slot, void *data ATTRIBUTE_UNUSED)
 {
   struct reduction_info *const red = *slot;
-  gimple_set_uid (red->reduc_phi, red->reduc_version);
+  gimple_set_uid (red->reduc_phi (), red->reduc_version);
   return 1;
 }
 
@@ -3559,7 +3569,7 @@ try_create_reduction_list (loop_p loop,
          if (dump_file && (dump_flags & TDF_DETAILS))
            {
              fprintf (dump_file, "reduction phi is  ");
-             print_gimple_stmt (dump_file, red->reduc_phi, 0);
+             print_gimple_stmt (dump_file, red->reduc_phi (), 0);
              fprintf (dump_file, "reduction stmt is  ");
              print_gimple_stmt (dump_file, red->reduc_stmt, 0);
            }
index 48d153d794a9b807585d11bb453360c1405dbdc9..a64066a9ae4faa33771a758c1563e5fd132fc42e 100644 (file)
@@ -622,6 +622,7 @@ split_loop (class loop *loop1)
        gphi *phi = find_or_create_guard_phi (loop1, guard_iv, &iv);
        if (!phi)
          continue;
+       const tree phi_result = gimple_phi_result (phi);
        gcond *guard_stmt = as_a<gcond *> (*gsi_last_bb (bbs[i]));
        tree guard_init = PHI_ARG_DEF_FROM_EDGE (phi,
                                                 loop_preheader_edge (loop1));
@@ -703,6 +704,12 @@ split_loop (class loop *loop1)
                                          profile_probability::very_likely (),
                                          true);
        gcc_assert (loop2);
+
+       /* The phi address may have changed due to being resized in
+          loop_version (), so reobtain it.  */
+       phi = as_a<gphi *> (SSA_NAME_DEF_STMT (phi_result));
+       gcc_checking_assert (gimple_bb (phi) == loop1->header);
+
        /* Correct probability of edge  cond_bb->preheader_of_loop2.  */
        single_pred_edge
                (loop_preheader_edge (loop2)->src)->probability
index 54b29540eb5aba428f21fbcb9b6d2180fb6f6b97..4f3abb94e6aa4c161c23c7ca3adf398aa1f1dac3 100644 (file)
@@ -4190,6 +4190,14 @@ vect_loop_versioning (loop_vec_info loop_vinfo,
                            prob * prob2, (prob * prob2).invert (),
                            prob * prob2, (prob * prob2).invert (),
                            true);
+
+      /* If the PHI nodes in the loop header were reallocated, we need to fix up
+        our internally stashed copies of those.  */
+      if (loop_to_version == loop)
+       for (auto gsi = gsi_start_phis (loop->header);
+            !gsi_end_p (gsi); gsi_next (&gsi))
+         loop_vinfo->resync_stmt_addr (gsi.phi ());
+
       /* We will later insert second conditional so overall outcome of
         both is prob * prob2.  */
       edge true_e, false_e;
index 0bec28048f4d09f7e97b3c74c85cd54b1c08d5ce..92ecd881508796bb1ad91494bc09e218cf56878c 100644 (file)
@@ -540,6 +540,29 @@ vec_info::add_pattern_stmt (gimple *stmt, stmt_vec_info stmt_info)
   return res;
 }
 
+/* If STMT was previously associated with a stmt_vec_info and STMT now resides
+   at a different address than before (e.g., because STMT is a phi node that has
+   been resized), update the stored address to match the new one.  It is not
+   possible to use lookup_stmt () to perform this task, because that function
+   returns NULL if the stored stmt pointer does not match the one being looked
+   up.  */
+
+stmt_vec_info
+vec_info::resync_stmt_addr (gimple *stmt)
+{
+  unsigned int uid = gimple_uid (stmt);
+  if (uid > 0 && uid - 1 < stmt_vec_infos.length ())
+    {
+      stmt_vec_info res = stmt_vec_infos[uid - 1];
+      if (res && res->stmt)
+       {
+         res->stmt = stmt;
+         return res;
+       }
+    }
+  return nullptr;
+}
+
 /* If STMT has an associated stmt_vec_info, return that vec_info, otherwise
    return null.  It is safe to call this function on any statement, even if
    it might not be part of the vectorizable region.  */
index 1059fc5c18f5fd80eea291a34ba6007384d97ddb..56fce666825b08ce6e6f9714d498239d9b3901f5 100644 (file)
@@ -497,6 +497,7 @@ public:
 
   stmt_vec_info add_stmt (gimple *);
   stmt_vec_info add_pattern_stmt (gimple *, stmt_vec_info);
+  stmt_vec_info resync_stmt_addr (gimple *);
   stmt_vec_info lookup_stmt (gimple *);
   stmt_vec_info lookup_def (tree);
   stmt_vec_info lookup_single_use (tree);