This patch is my new way to handle what was previously done in v2 patch
04/14, discussed at:
https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669527.html
The only places I have found running into trouble with reallocated PHI nodes
(at least, the only places revealed by changing the size of location_t) are
those which remember a gphi object before a call to loop_version() and then
try to use it after. I fixed three affected call sites in this new patch.
-- >8 --
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 | 40 +++++++++++++++++++++++--------------
gcc/tree-ssa-loop-split.cc | 7 +++++++
gcc/tree-vect-loop-manip.cc | 8 ++++++++
gcc/tree-vectorizer.cc | 20 +++++++++++++++++++
gcc/tree-vectorizer.h | 1 +
5 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/gcc/tree-parloops.cc b/gcc/tree-parloops.cc
index 13d8e84bc8f..8427c287a6a 100644
--- a/gcc/tree-parloops.cc
+++ b/gcc/tree-parloops.cc
@@ -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);
}
diff --git a/gcc/tree-ssa-loop-split.cc b/gcc/tree-ssa-loop-split.cc
index 48d153d794a..a64066a9ae4 100644
--- a/gcc/tree-ssa-loop-split.cc
+++ b/gcc/tree-ssa-loop-split.cc
@@ -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
diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
index 54b29540eb5..4f3abb94e6a 100644
--- a/gcc/tree-vect-loop-manip.cc
+++ b/gcc/tree-vect-loop-manip.cc
@@ -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;
diff --git a/gcc/tree-vectorizer.cc b/gcc/tree-vectorizer.cc
index 9a068a40864..c8bb0fe14fc 100644
--- a/gcc/tree-vectorizer.cc
+++ b/gcc/tree-vectorizer.cc
@@ -541,6 +541,26 @@ 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 expected address to match the new one. */
+
+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. */
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 7f69a3f57b4..749a9299d35 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -500,6 +500,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);
