On Fri, Nov 17, 2023 at 9:45 AM Robin Dapp <rdapp....@gmail.com> wrote: > > > But note you can explicitly specify a vector type as well, there's an > > overload for it, so we can fix the "invariant" case with the following > > (OK if you can test this on relevant targets) > > > > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc > > index 3f59139cb01..936a3de9534 100644 > > --- a/gcc/tree-vect-loop.cc > > +++ b/gcc/tree-vect-loop.cc > > @@ -8450,12 +8450,14 @@ vect_transform_reduction (loop_vec_info loop_vinfo, > > value. */ > > bool cond_fn_p = code.is_internal_fn () > > && conditional_internal_fn_code (internal_fn (code)) != ERROR_MARK; > > + int cond_index = -1; > > if (cond_fn_p) > > { > > gcc_assert (code == IFN_COND_ADD || code == IFN_COND_SUB > > || code == IFN_COND_MUL || code == IFN_COND_AND > > || code == IFN_COND_IOR || code == IFN_COND_XOR); > > gcc_assert (op.num_ops == 4 && (op.ops[1] == op.ops[3])); > > + cond_index = 0; > > } > > > > bool masked_loop_p = LOOP_VINFO_FULLY_MASKED_P (loop_vinfo); > > @@ -8486,12 +8488,13 @@ vect_transform_reduction (loop_vec_info loop_vinfo, > > vect_get_vec_defs (loop_vinfo, stmt_info, slp_node, ncopies, > > single_defuse_cycle && reduc_index == 0 > > ? NULL_TREE : op.ops[0], &vec_oprnds0, > > + cond_index == 0 ? truth_type_for (vectype_in) : > > NULL_TREE, > > single_defuse_cycle && reduc_index == 1 > > - ? NULL_TREE : op.ops[1], &vec_oprnds1, > > + ? NULL_TREE : op.ops[1], &vec_oprnds1, NULL_TREE, > > op.num_ops == 4 > > || (op.num_ops == 3 > > && !(single_defuse_cycle && reduc_index == 2)) > > - ? op.ops[2] : NULL_TREE, &vec_oprnds2); > > + ? op.ops[2] : NULL_TREE, &vec_oprnds2, NULL_TREE); > > Ah, yes that's what I meant. I had something along those lines: > > + if (!cond_fn_p) > + { > + vect_get_vec_defs (loop_vinfo, stmt_info, slp_node, ncopies, > + single_defuse_cycle && reduc_index == 0 > + ? NULL_TREE : op.ops[0], &vec_oprnds0, > + single_defuse_cycle && reduc_index == 1 > + ? NULL_TREE : op.ops[1], &vec_oprnds1, > + op.num_ops == 3 > + && !(single_defuse_cycle && reduc_index == 2) > + ? op.ops[2] : NULL_TREE, &vec_oprnds2); > + } > + else > + { > + /* For a conditional operation, pass the truth type as vectype for the > + mask. */ > + gcc_assert (single_defuse_cycle && reduc_index == 1); > + vect_get_vec_defs (loop_vinfo, stmt_info, slp_node, ncopies, > + op.ops[0], &vec_oprnds0, > + truth_type_for (vectype_in), > + NULL_TREE, &vec_oprnds1, NULL_TREE, > + op.ops[2], &vec_oprnds2, NULL_TREE); > + } > > Even though this has a bit of duplication now I prefer it slightly > because of the. I'd hope, once fully tested (it's already running > but I'm having connection problems so don't know about the results > yet), this could go in independently of the pattern fix?
Yes, your version is also OK. Thanks, Richard. > > Regards > Robin >