https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127410
Bug ID: 127410
Summary: SLP node data recording is not idempotent; re-analysis
discards the recorded result
Product: gcc
Version: 17.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: ptomsich at gcc dot gnu.org
Target Milestone: ---
vect_slp_analyze_node_operations adds each node to visited_set, but when
analysis fails it pops the whole recursion back off again:
if (!res)
{
while (visited_vec.length () >= visited_rec_start)
visited_set.remove (visited_vec.pop ());
cost_vec->truncate (cost_vec_rec_start);
}
Nodes popped there can already carry analysis data: vect_slp_analyze_node_
operations_1 ran for them before a later sibling failed. Once they are out
of visited_set, a later traversal analyzes them again.
The recording is not written for that:
SLP_TREE_TYPE (slp_node) = call_simd_clone_vec_info_type;
slp_node->data = new vect_simd_clone_data (std::move (_data));
On the second analysis get_data returns the object installed by the first, so
every update above lands in it, and this store then replaces it with the
untouched local _data. For simd clones that is a correctness problem.
I have no testcase that reaches this on unmodified trunk. It was found with
some experimental work (for astcenc_r) that retries cost-rejected BB-SLP
subgraphs with shared children demoted (and re-analyzing): we were working to
address that vect_bb_partition_graph fuses SLP instances that share nodes into
one subgraph with an all-or-nothing cost verdict ... for this, we salvage an
instance out of a rejected subgraph and rscore after editing operands.
The open design question is: is re-analysis allowed or is a node analyzed at
most once? If re-analysis is allowed (and that's how I read it), then the
recording has to be idempotent with a change like the following:
@@ vectorizable_simd_clone_call
data.clone = bestn;
data.clone_inbranch = bestn_inbranch;
+ simd_clone_info.truncate (0);
simd_clone_info.safe_push (NULL_TREE);
@@
SLP_TREE_TYPE (slp_node) = call_simd_clone_vec_info_type;
- slp_node->data = new vect_simd_clone_data (std::move (_data));
+ if (!slp_node->data)
+ slp_node->data = new vect_simd_clone_data (std::move (_data));
@@ struct vect_simd_clone_data
- cgraph_node *clone;
- cgraph_node *clone_inbranch;
+ cgraph_node *clone = nullptr;
+ cgraph_node *clone_inbranch = nullptr;
