[gcc r17-3499] choose VF independent of externs/constants
Richard Biener via Gcc-cvs <[email protected]>
| Newsgroups | gmane.comp.gcc.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://gcc.gnu.org/g:bb1344be44e364da8377603697933a668b651c4e commit r17-3499-gbb1344be44e364da8377603697933a668b651c4e Author: Richard Biener <[email protected]> Date: Wed Aug 19 14:50:09 2026 +0200 choose VF independent of externs/constants The following decides on the VF based on assigned SLP_TREE_VECTYPE rather than on the tracked max_nunits in the SLP graph which also factors in external and constant nodes. Those get their vector type assigned in vectorizable_* via vect_maybe_update_slp_op_vectype and there's freedom to vectorizable_* to pick a suitable one. Specifically conversions of externs/constants can cause a higher than necessary VF which would be a missed optimization. Changing the VF as outlined will run into both vect_prologue_cost_for_slp and vect_create_constant_vectors ICEing when computing vect_get_num_copies since that asserts it can exact_div the number of lanes as in the unrolled loop by the number of lanes in the (out of loop) vector type. But vectorizable_* can opt to just use the lowpart of such vectors but require it in full due to target constraints. So this RFC patch introduces vect_get_num_copies_for_invariant which performs a ceil_div instead (I think we can maybe change the global copy to do that). I added verification to vect_slp_analyze_node_operations which should make the vect_maybe_update_slp_op_vectype change in the predicated tails series unneeded. This shifts the ICE to vectorizable_conversion which is mightly confused by invariant vectors with too many lanes (in O3-vect-pr32243.c it is unswitching that exposes a loop invariant conversion in the loop). I have installed a narrow fix that is likely incomplete and I expect similar issues to appear in multi-operand widening/narrowing operations. The fundamental issue is that we expose constant/externals to vectorizable_* that might have an excess number of lanes, and some vectorizable_* might not be prepared (and most should be unaffected). As vectorizable_* control the vector types themselves the fix lies within them. Once fuzzers increase coverage. * tree-vect-slp.cc (vect_update_slp_vf_for_node): Compute VF based on SLP_TREE_VECTYPE only. (vect_get_num_copies_for_invariant): New. (vect_prologue_cost_for_slp): Take nvectors as argument. (vect_slp_analyze_node_operations): Verify that we can div_away_from_zero for vect_prologue_cost_for_slp. (vect_create_constant_vectors): Use vect_get_num_copies_for_invariant. * tree-vect-stmts.cc (vect_create_vectorized_promotion_stmts): Support creating half of the promoted results. (vectorizable_conversion): When we have excess input elements request half of the promoted results. Diff: --- gcc/tree-vect-slp.cc | 52 +++++++++++++++++++++++++++++++++++++++----------- gcc/tree-vect-stmts.cc | 43 ++++++++++++++++++++++++----------------- 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index 9dce9208626e..a2b679df9700 100644 --- a/gcc/tree-vect-slp.cc +++ b/gcc/tree-vect-slp.cc @@ -8682,16 +8682,19 @@ vect_update_slp_vf_for_node (slp_tree node, poly_uint64 &vf, vect_update_slp_vf_for_node (child, vf, visited); /* We do not visit SLP nodes for constants or externals - those neither - have a vector type set yet (vectorizable_* does this) nor do they - have max_nunits set. Instead we rely on internal nodes max_nunit - to cover constant/external operands. + have a vector type set yet (vectorizable_* does this). Note that when we stop using fixed size vectors externs and constants shouldn't influence the (minimum) vectorization factor, instead vectorizable_* should honor the vectorization factor when trying to assign vector types to constants and externals and cause iteration to a higher vectorization factor when required. */ + tree vectype = SLP_TREE_VECTYPE (node); + if (!vectype) + /* OMP SIMD calls w/o LHS have no SLP_TREE_VECTYPE set. */ + return; poly_uint64 node_vf - = calculate_unrolling_factor (node->max_nunits, SLP_TREE_LANES (node)); + = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype), + SLP_TREE_LANES (node)); vf = force_common_multiple (vf, node_vf); /* For permute nodes that are fed from externs or constants we have to @@ -8701,7 +8704,7 @@ vect_update_slp_vf_for_node (slp_tree node, poly_uint64 &vf, if (SLP_TREE_DEF_TYPE (child) != vect_internal_def) { poly_uint64 child_vf - = calculate_unrolling_factor (node->max_nunits, + = calculate_unrolling_factor (TYPE_VECTOR_SUBPARTS (vectype), SLP_TREE_LANES (child)); vf = force_common_multiple (vf, child_vf); } @@ -8998,11 +9001,33 @@ vect_scalar_ops_slice_hash::equal (const value_type &s1, return true; } +/* Like vect_get_num_copies but N copies of the vector might have + excess elements in the last vector. Returns false if *NVECTORS + cannot be computed. */ + +static bool +vect_get_num_copies_for_invariant (vec_info *vinfo, slp_tree node, + unsigned *nvectors) +{ + poly_uint64 vf; + + if (loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo)) + vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo); + else + vf = 1; + vf *= SLP_TREE_LANES (node); + + tree vectype = SLP_TREE_VECTYPE (node); + bool res = can_div_away_from_zero_p (vf, TYPE_VECTOR_SUBPARTS (vectype), + nvectors); + return res; +} + /* Compute the prologue cost for invariant or constant operands represented by NODE. */ static void -vect_prologue_cost_for_slp (vec_info *vinfo, slp_tree node, +vect_prologue_cost_for_slp (slp_tree node, unsigned nvectors, stmt_vector_for_cost *cost_vec) { /* There's a special case of an existing vector, that costs nothing. */ @@ -9016,7 +9041,6 @@ vect_prologue_cost_for_slp (vec_info *vinfo, slp_tree node, unsigned group_size = SLP_TREE_LANES (node); unsigned HOST_WIDE_INT const_nunits; unsigned nelt_limit; - unsigned nvectors = vect_get_num_copies (vinfo, node); auto ops = &SLP_TREE_SCALAR_OPS (node); auto_vec<unsigned int> starts (nvectors); if (TYPE_VECTOR_SUBPARTS (vectype).is_constant (&const_nunits) @@ -9075,7 +9099,7 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node, int i, j; slp_tree child; - /* Assume we can code-generate all invariants. */ + /* Costing and analysis of invariants is delayed. */ if (!node || SLP_TREE_DEF_TYPE (node) == vect_constant_def || SLP_TREE_DEF_TYPE (node) == vect_external_def) @@ -9178,8 +9202,11 @@ vect_slp_analyze_node_operations (vec_info *vinfo, slp_tree node, continue; } - /* And cost them. */ - vect_prologue_cost_for_slp (vinfo, child, cost_vec); + /* Make sure we can generate them and then cost them. */ + unsigned nvectors; + if (!vect_get_num_copies_for_invariant (vinfo, node, &nvectors)) + return false; + vect_prologue_cost_for_slp (child, nvectors, cost_vec); } /* If this node or any of its children can't be vectorized, try pruning @@ -10946,7 +10973,10 @@ vect_create_constant_vectors (vec_info *vinfo, slp_tree op_node) /* We always want SLP_TREE_VECTYPE (op_node) here correctly set. */ vector_type = SLP_TREE_VECTYPE (op_node); - unsigned int number_of_vectors = vect_get_num_copies (vinfo, op_node); + unsigned int number_of_vectors; + bool res = vect_get_num_copies_for_invariant (vinfo, op_node, + &number_of_vectors); + gcc_assert (res); SLP_TREE_VEC_DEFS (op_node).create (number_of_vectors); auto_vec<tree> voprnds (number_of_vectors); diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index 38a17e29a84c..a8a2ec62920d 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -5160,7 +5160,8 @@ vect_create_vectorized_demotion_stmts (vec_info *vinfo, vec<tree> *vec_oprnds, /* Create vectorized promotion statements for vector operands from VEC_OPRNDS0 and VEC_OPRNDS1, for a binary operation associated with scalar statement STMT_INFO. For multi-step conversions store the resulting vectors and - call the function recursively. */ + call the function recursively. When HALF is true only generate half + of the result. */ static void vect_create_vectorized_promotion_stmts (vec_info *vinfo, @@ -5169,14 +5170,15 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo, stmt_vec_info stmt_info, tree vec_dest, gimple_stmt_iterator *gsi, code_helper ch1, - code_helper ch2, int op_type) + code_helper ch2, int op_type, + bool half) { int i; tree vop0, vop1, new_tmp1, new_tmp2; gimple *new_stmt1, *new_stmt2; vec<tree> vec_tmp = vNULL; - vec_tmp.create (vec_oprnds0->length () * 2); + vec_tmp.create ((half ? 1 : 2) * vec_oprnds0->length ()); FOR_EACH_VEC_ELT (*vec_oprnds0, i, vop0) { if (op_type == binary_op) @@ -5188,23 +5190,20 @@ vect_create_vectorized_promotion_stmts (vec_info *vinfo, new_stmt1 = vect_gen_widened_results_half (vinfo, ch1, vop0, vop1, op_type, vec_dest, gsi, stmt_info); - new_stmt2 = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1, - op_type, vec_dest, gsi, - stmt_info); - if (is_gimple_call (new_stmt1)) - { - new_tmp1 = gimple_call_lhs (new_stmt1); - new_tmp2 = gimple_call_lhs (new_stmt2); - } - else + new_tmp1 = gimple_get_lhs (new_stmt1); + vec_tmp.quick_push (new_tmp1); + + if (vec_tmp.space (1)) { - new_tmp1 = gimple_assign_lhs (new_stmt1); - new_tmp2 = gimple_assign_lhs (new_stmt2); + new_stmt2 = vect_gen_widened_results_half (vinfo, ch2, vop0, vop1, + op_type, vec_dest, gsi, + stmt_info); + new_tmp2 = gimple_get_lhs (new_stmt2); + vec_tmp.quick_push (new_tmp2); } - /* Store the results for the next step. */ - vec_tmp.quick_push (new_tmp1); - vec_tmp.quick_push (new_tmp2); + if (!vec_tmp.space (1)) + break; } vec_oprnds0->release (); @@ -5760,6 +5759,7 @@ vectorizable_conversion (vec_info *vinfo, widen_or_narrow_float_p ? vectype_out : cvt_type); + unsigned num_vectors = vect_get_num_copies (vinfo, slp_node); switch (modifier) { case NONE: @@ -5843,10 +5843,17 @@ vectorizable_conversion (vec_info *vinfo, stmt_info, this_dest, gsi, c1, op_type); else + /* ??? For constant/external inputs we can end up with + excess lanes. When the number of inputs already match + the number of required outputs request half of the + lanes (gcc.dg/vect/O3-vect-pr32243.c). Low coverage + makes this likely incomplete. */ vect_create_vectorized_promotion_stmts (vinfo, &vec_oprnds0, &vec_oprnds1, stmt_info, this_dest, gsi, - c1, c2, op_type); + c1, c2, op_type, + vec_oprnds0.length () + == num_vectors); } FOR_EACH_VEC_ELT (vec_oprnds0, i, vop0)