On 25/04/2023 13:30, Richard Biener wrote:
On Mon, 24 Apr 2023, Richard Sandiford wrote:
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.
Yeah, I was hoping to have a look whether the new gimple_build
overloads could be used to make this all better (but hoped we can
finally get this series in in some way).
Richard.
Yeah, in the newest version of the first patch of the series I found
that most of the time I can get away with only really needing to
distinguish between tree_code and internal_fn when building gimple, for
which it currently uses vect_gimple_build, but it does feel like that
could easily be a gimple function.
Having said that, as I partially mention in the patch, I didn't rewrite
the optabs-tree supportable_half_widening and supportable_conversion (or
whatever they are called) because those also at some point need to
access the stmt and there is a massive difference in how we handle
gassigns and gcall's from that perspective, but maybe we can generalize
that too somehow...
Anyway have a look at the new versions (posted just some minutes after
the email I'm replying too haha! timing :P)
