Hi Richard,
Thank you for the review, I've adopted all above suggestions downstream,
I am still surprised how many style things I still miss after years of
gcc development :(
On 17/12/2021 12:44, Richard Sandiford wrote:
@@ -3252,16 +3257,31 @@ vectorizable_call (vec_info *vinfo,
rhs_type = unsigned_type_node;
}
- int mask_opno = -1;
+ /* The argument that is not of the same type as the others. */
+ int diff_opno = -1;
+ bool masked = false;
if (internal_fn_p (cfn))
- mask_opno = internal_fn_mask_index (as_internal_fn (cfn));
+ {
+ if (cfn == CFN_FTRUNC_INT)
+ /* For FTRUNC this represents the argument that carries the type of the
+ intermediate signed integer. */
+ diff_opno = 1;
+ else
+ {
+ /* For masked operations this represents the argument that carries the
+ mask. */
+ diff_opno = internal_fn_mask_index (as_internal_fn (cfn));
+ masked = diff_opno >= 0;
+ }
+ }
I think it would be cleaner not to process argument 1 at all for
CFN_FTRUNC_INT. There's no particular need to vectorise it.
I agree with this, will change the loop to continue for argument 1 when
dealing with non-masked CFN's.
}
[…]
diff --git a/gcc/tree.c b/gcc/tree.c
index
845228a055b2cfac0c9ca8c0cda1b9df4b0095c6..f1e9a1eb48769cb11aa69730e2480ed5522f78c1
100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -6645,11 +6645,11 @@ valid_constant_size_p (const_tree size, cst_size_error
*perr /* = NULL */)
return true;
}
-/* Return the precision of the type, or for a complex or vector type the
- precision of the type of its elements. */
+/* Return the type, or for a complex or vector type the type of its
+ elements. */
-unsigned int
-element_precision (const_tree type)
+tree
+element_type (const_tree type)
{
if (!TYPE_P (type))
type = TREE_TYPE (type);
@@ -6657,7 +6657,16 @@ element_precision (const_tree type)
if (code == COMPLEX_TYPE || code == VECTOR_TYPE)
type = TREE_TYPE (type);
- return TYPE_PRECISION (type);
+ return (tree) type;
I think we should stick a const_cast in element_precision and make
element_type take a plain “type”. As it stands element_type is an
implicit const_cast for many cases.
Thanks,
Was just curious about something here, I thought the purpose of having
element_precision (before) and element_type (now) take a const_tree as
an argument was to make it clear we aren't changing the input type. I
understand that as it stands element_type could be an implicit
const_cast (which I should be using rather than the '(tree)' cast), but
that's only if 'type' is a type that isn't complex/vector, either way,
we are conforming to the promise that we aren't changing the incoming
type, what the caller then does with the result is up to them no?
I don't mind making the changes, just trying to understand the reasoning
behind it.
I'll send in a new patch with all the changes after the review on the
match.pd stuff.
Thanks,
Andre
