Richard Biener <richard.guent...@gmail.com> writes: > On Thu, Apr 20, 2023 at 3:24 PM Andre Vieira (lists) via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> Rebased all three patches and made some small changes to the second one: >> - removed sub and abd optabs from commutative_optab_p, I suspect this >> was a copy paste mistake, >> - removed what I believe to be a superfluous switch case in vectorizable >> conversion, the one that was here: >> + if (code.is_fn_code ()) >> + { >> + internal_fn ifn = as_internal_fn (code.as_fn_code ()); >> + int ecf_flags = internal_fn_flags (ifn); >> + gcc_assert (ecf_flags & ECF_MULTI); >> + >> + switch (code.as_fn_code ()) >> + { >> + case CFN_VEC_WIDEN_PLUS: >> + break; >> + case CFN_VEC_WIDEN_MINUS: >> + break; >> + case CFN_LAST: >> + default: >> + return false; >> + } >> + >> + internal_fn lo, hi; >> + lookup_multi_internal_fn (ifn, &lo, &hi); >> + *code1 = as_combined_fn (lo); >> + *code2 = as_combined_fn (hi); >> + optab1 = lookup_multi_ifn_optab (lo, !TYPE_UNSIGNED (vectype)); >> + optab2 = lookup_multi_ifn_optab (hi, !TYPE_UNSIGNED (vectype)); >> } >> >> I don't think we need to check they are a specfic fn code, as we look-up >> optabs and if they succeed then surely we can vectorize? >> >> OK for trunk? > > In the first patch I see some uses of safe_as_tree_code like > > + if (ch.is_tree_code ()) > + return op1 == NULL_TREE ? gimple_build_assign (lhs, > ch.safe_as_tree_code (), > + op0) : > + gimple_build_assign (lhs, ch.safe_as_tree_code > (), > + op0, op1); > + else > + { > + internal_fn fn = as_internal_fn (ch.safe_as_fn_code ()); > + gimple* stmt; > > where the context actually requires a valid tree code. Please change those > to force to tree code / ifn code. Just use explicit casts here and the other > places that are similar. Before the as_internal_fn just put a > gcc_assert (ch.is_internal_fn ()).
Also, doesn't the above ?: simplify to the "else" arm? Null trailing arguments would be ignored for unary operators. I wasn't sure what to make of the op0 handling: > +/* Build a GIMPLE_ASSIGN or GIMPLE_CALL with the tree_code, > + or internal_fn contained in ch, respectively. */ > +gimple * > +vect_gimple_build (tree lhs, code_helper ch, tree op0, tree op1) > +{ > + if (op0 == NULL_TREE) > + return NULL; Can that happen, and if so, does returning null make sense? Maybe an assert would be safer. Thanks, Richard