On 11/07/2024 22:42, Christophe Lyon wrote:
+ bool
+ check (function_checker &c) const override
+ {
+ if (c.mode_suffix_id == MODE_none)
+ return true;
+
+ unsigned int bits = c.type_suffix (0).element_bits;
+ return c.require_immediate_range (1, 1, bits);
+ }
When trying to understand how this worked I bumped into the following:
If you pass a value that's not in the appropriate range like 33, you'll see:
vcvt.c:5:10: error: passing 33 to argument 2 of 'vcvtq_n', which expects
a value in the range [1, 32]
If you however pick a negative number like -3? You get:
vcvt.c:5:10: error: argument 2 of 'vcvtq_n' must be an integer constant
expression
This is somewhat confusing and yes we could change the message to say
'must be a positive integer constant expression', which might be clear
enough in this case, but less so if you do something like 1<<7 for this
intrinsic, because the signature looks like:
> + build_all (b, "v0,v1,ss8", group, MODE_n, preserve_user_namespace);
which converts the immediate to a signed 8-bit value, making 1<<7 a
negative number, and the intrinsic specs defines the parameter as a
signed 32-bit integer. So if a user accidentally passes 1<<7 here, they
will get:
vcvt.c:5:10: error: argument 2 of 'vcvtq_n' must be an integer constant
expression
Potentially making users think GCC does not support inline evaluation
for these parameters, which would be sad.
So we should at least change the signature to be "v0,v1,ss32", but I
suggest we deviate and make the last parameter in the signature either a
su32 or a su64. As the framework code expects this value to fit into a
uhwi, which I suspect is because SVE always defines the parameters as
uint64_t constants, i.e. svxar_n_s64, disclaimer I didn't check all SVE
intrinsics.
Initially I was going to suggest fold_converting the arg in
'function_checker::require_immediate' into a
long_long_unsigned_type_node, which also does what we want in these
examples, but perhaps going the signature way is cleaner and more inline
with the framework as is. We probably want to keep it as much as SVE as
possible, so we can 'borrow' code more easily if the need arises.
@Richard S: I added you to this review as you know the SVE intrinsic
code better than most (if not all). So hopefully you can let us know if
you have an opinion on this.
Kind Regards,
Andre
