diff mbox series

[v2,22/36] arm: [MVE intrinsics] fix checks of immediate arguments

Message ID 20240904132650.2720446-23-christophe.lyon@linaro.org
State New
Headers show
Series arm: [MVE intrinsics] Re-implement more intrinsics | expand

Commit Message

Christophe Lyon Sept. 4, 2024, 1:26 p.m. UTC
As discussed in [1], it is better to use "su64" for immediates in
intrinsics signatures in order to provide better diagnostics
(erroneous constants are not truncated for instance).  This patch thus
uses su64 instead of ss32 in binary_lshift_unsigned,
binary_rshift_narrow, binary_rshift_narrow_unsigned, ternary_lshift,
ternary_rshift.

In addition, we fix cases where we called require_integer_immediate
whereas we just want to check that the argument is a scalar, and thus
use require_scalar_type in binary_acca_int32, binary_acca_int64,
unary_int32_acc.

Finally, in binary_lshift_unsigned we just want to check that 'imm' is
an immediate, not the optional predicates.

[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660262.html

2024-08-21  Christophe Lyon  <christophe.lyon@linaro.org>

	gcc/
	* config/arm/arm-mve-builtins-shapes.cc (binary_acca_int32): Fix
	check of scalar argument.
	(binary_acca_int64): Likewise.
	(binary_lshift_unsigned): Likewise.
	(binary_rshift_narrow): Likewise.
	(binary_rshift_narrow_unsigned): Likewise.
	(ternary_lshift): Likewise.
	(ternary_rshift): Likewise.
	(unary_int32_acc): Likewise.
---
 gcc/config/arm/arm-mve-builtins-shapes.cc | 47 +++++++++++++++--------
 1 file changed, 31 insertions(+), 16 deletions(-)

Comments

Richard Earnshaw (lists) Oct. 14, 2024, 5:43 p.m. UTC | #1
On 04/09/2024 14:26, Christophe Lyon wrote:
> As discussed in [1], it is better to use "su64" for immediates in
> intrinsics signatures in order to provide better diagnostics
> (erroneous constants are not truncated for instance).  This patch thus
> uses su64 instead of ss32 in binary_lshift_unsigned,
> binary_rshift_narrow, binary_rshift_narrow_unsigned, ternary_lshift,
> ternary_rshift.
> 
> In addition, we fix cases where we called require_integer_immediate
> whereas we just want to check that the argument is a scalar, and thus
> use require_scalar_type in binary_acca_int32, binary_acca_int64,
> unary_int32_acc.
> 
> Finally, in binary_lshift_unsigned we just want to check that 'imm' is
> an immediate, not the optional predicates.
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660262.html
> 
> 2024-08-21  Christophe Lyon  <christophe.lyon@linaro.org>
> 
> 	gcc/
> 	* config/arm/arm-mve-builtins-shapes.cc (binary_acca_int32): Fix
> 	check of scalar argument.
> 	(binary_acca_int64): Likewise.
> 	(binary_lshift_unsigned): Likewise.
> 	(binary_rshift_narrow): Likewise.
> 	(binary_rshift_narrow_unsigned): Likewise.
> 	(ternary_lshift): Likewise.
> 	(ternary_rshift): Likewise.
> 	(unary_int32_acc): Likewise.

OK.

R.

> ---
>  gcc/config/arm/arm-mve-builtins-shapes.cc | 47 +++++++++++++++--------
>  1 file changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc
> index 971e86a2727..a1d2e243128 100644
> --- a/gcc/config/arm/arm-mve-builtins-shapes.cc
> +++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
> @@ -477,18 +477,23 @@ struct binary_acca_int32_def : public overloaded_base<0>
>    {
>      unsigned int i, nargs;
>      type_suffix_index type;
> +    const char *first_type_name;
> +
>      if (!r.check_gp_argument (3, i, nargs)
>  	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
>        return error_mark_node;
>  
> +    first_type_name = (type_suffixes[type].unsigned_p
> +		       ? "uint32_t"
> +		       : "int32_t");
> +    if (!r.require_scalar_type (0, first_type_name))
> +      return error_mark_node;
> +
>      unsigned int last_arg = i + 1;
>      for (i = 1; i < last_arg; i++)
>        if (!r.require_matching_vector_type (i, type))
>  	return error_mark_node;
>  
> -    if (!r.require_integer_immediate (0))
> -      return error_mark_node;
> -
>      return r.resolve_to (r.mode_suffix_id, type);
>    }
>  };
> @@ -514,18 +519,24 @@ struct binary_acca_int64_def : public overloaded_base<0>
>    {
>      unsigned int i, nargs;
>      type_suffix_index type;
> +    const char *first_type_name;
> +
>      if (!r.check_gp_argument (3, i, nargs)
>  	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
>        return error_mark_node;
>  
> +
> +    first_type_name = (type_suffixes[type].unsigned_p
> +		       ? "uint64_t"
> +		       : "int64_t");
> +    if (!r.require_scalar_type (0, first_type_name))
> +      return error_mark_node;
> +
>      unsigned int last_arg = i + 1;
>      for (i = 1; i < last_arg; i++)
>        if (!r.require_matching_vector_type (i, type))
>  	return error_mark_node;
>  
> -    if (!r.require_integer_immediate (0))
> -      return error_mark_node;
> -
>      return r.resolve_to (r.mode_suffix_id, type);
>    }
>  };
> @@ -613,7 +624,7 @@ struct binary_lshift_unsigned_def : public overloaded_base<0>
>  	 bool preserve_user_namespace) const override
>    {
>      b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
> -    build_all (b, "vu0,vs0,ss32", group, MODE_n, preserve_user_namespace);
> +    build_all (b, "vu0,vs0,su64", group, MODE_n, preserve_user_namespace);
>    }
>  
>    tree
> @@ -622,6 +633,7 @@ struct binary_lshift_unsigned_def : public overloaded_base<0>
>      unsigned int i, nargs;
>      type_suffix_index type;
>      if (!r.check_gp_argument (2, i, nargs)
> +	|| !r.require_integer_immediate (i)
>  	|| (type = r.infer_vector_type (i-1)) == NUM_TYPE_SUFFIXES)
>        return error_mark_node;
>  
> @@ -636,10 +648,6 @@ struct binary_lshift_unsigned_def : public overloaded_base<0>
>  	  return error_mark_node;
>        }
>  
> -    for (; i < nargs; ++i)
> -      if (!r.require_integer_immediate (i))
> -	return error_mark_node;
> -
>      return r.resolve_to (r.mode_suffix_id, type);
>    }
>  
> @@ -1097,7 +1105,7 @@ struct binary_rshift_narrow_def : public overloaded_base<0>
>  	 bool preserve_user_namespace) const override
>    {
>      b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
> -    build_all (b, "vh0,vh0,v0,ss32", group, MODE_n, preserve_user_namespace);
> +    build_all (b, "vh0,vh0,v0,su64", group, MODE_n, preserve_user_namespace);
>    }
>  
>    tree
> @@ -1144,7 +1152,7 @@ struct binary_rshift_narrow_unsigned_def : public overloaded_base<0>
>  	 bool preserve_user_namespace) const override
>    {
>      b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
> -    build_all (b, "vhu0,vhu0,v0,ss32", group, MODE_n, preserve_user_namespace);
> +    build_all (b, "vhu0,vhu0,v0,su64", group, MODE_n, preserve_user_namespace);
>    }
>  
>    tree
> @@ -1588,7 +1596,7 @@ struct ternary_lshift_def : public overloaded_base<0>
>  	 bool preserve_user_namespace) const override
>    {
>      b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
> -    build_all (b, "v0,v0,v0,ss32", group, MODE_n, preserve_user_namespace);
> +    build_all (b, "v0,v0,v0,su64", group, MODE_n, preserve_user_namespace);
>    }
>  
>    tree
> @@ -1683,7 +1691,7 @@ struct ternary_rshift_def : public overloaded_base<0>
>  	 bool preserve_user_namespace) const override
>    {
>      b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
> -    build_all (b, "v0,v0,v0,ss32", group, MODE_n, preserve_user_namespace);
> +    build_all (b, "v0,v0,v0,su64", group, MODE_n, preserve_user_namespace);
>    }
>  
>    tree
> @@ -1838,11 +1846,18 @@ struct unary_int32_acc_def : public overloaded_base<0>
>    {
>      unsigned int i, nargs;
>      type_suffix_index type;
> +    const char *first_type_name;
> +
>      if (!r.check_gp_argument (2, i, nargs)
> -	|| !r.require_integer_immediate (0)
>  	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
>        return error_mark_node;
>  
> +    first_type_name = (type_suffixes[type].unsigned_p
> +		       ? "uint32_t"
> +		       : "int32_t");
> +    if (!r.require_scalar_type (0, first_type_name))
> +      return error_mark_node;
> +
>      return r.resolve_to (r.mode_suffix_id, type);
>    }
>  };
diff mbox series

Patch

diff --git a/gcc/config/arm/arm-mve-builtins-shapes.cc b/gcc/config/arm/arm-mve-builtins-shapes.cc
index 971e86a2727..a1d2e243128 100644
--- a/gcc/config/arm/arm-mve-builtins-shapes.cc
+++ b/gcc/config/arm/arm-mve-builtins-shapes.cc
@@ -477,18 +477,23 @@  struct binary_acca_int32_def : public overloaded_base<0>
   {
     unsigned int i, nargs;
     type_suffix_index type;
+    const char *first_type_name;
+
     if (!r.check_gp_argument (3, i, nargs)
 	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
       return error_mark_node;
 
+    first_type_name = (type_suffixes[type].unsigned_p
+		       ? "uint32_t"
+		       : "int32_t");
+    if (!r.require_scalar_type (0, first_type_name))
+      return error_mark_node;
+
     unsigned int last_arg = i + 1;
     for (i = 1; i < last_arg; i++)
       if (!r.require_matching_vector_type (i, type))
 	return error_mark_node;
 
-    if (!r.require_integer_immediate (0))
-      return error_mark_node;
-
     return r.resolve_to (r.mode_suffix_id, type);
   }
 };
@@ -514,18 +519,24 @@  struct binary_acca_int64_def : public overloaded_base<0>
   {
     unsigned int i, nargs;
     type_suffix_index type;
+    const char *first_type_name;
+
     if (!r.check_gp_argument (3, i, nargs)
 	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
       return error_mark_node;
 
+
+    first_type_name = (type_suffixes[type].unsigned_p
+		       ? "uint64_t"
+		       : "int64_t");
+    if (!r.require_scalar_type (0, first_type_name))
+      return error_mark_node;
+
     unsigned int last_arg = i + 1;
     for (i = 1; i < last_arg; i++)
       if (!r.require_matching_vector_type (i, type))
 	return error_mark_node;
 
-    if (!r.require_integer_immediate (0))
-      return error_mark_node;
-
     return r.resolve_to (r.mode_suffix_id, type);
   }
 };
@@ -613,7 +624,7 @@  struct binary_lshift_unsigned_def : public overloaded_base<0>
 	 bool preserve_user_namespace) const override
   {
     b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
-    build_all (b, "vu0,vs0,ss32", group, MODE_n, preserve_user_namespace);
+    build_all (b, "vu0,vs0,su64", group, MODE_n, preserve_user_namespace);
   }
 
   tree
@@ -622,6 +633,7 @@  struct binary_lshift_unsigned_def : public overloaded_base<0>
     unsigned int i, nargs;
     type_suffix_index type;
     if (!r.check_gp_argument (2, i, nargs)
+	|| !r.require_integer_immediate (i)
 	|| (type = r.infer_vector_type (i-1)) == NUM_TYPE_SUFFIXES)
       return error_mark_node;
 
@@ -636,10 +648,6 @@  struct binary_lshift_unsigned_def : public overloaded_base<0>
 	  return error_mark_node;
       }
 
-    for (; i < nargs; ++i)
-      if (!r.require_integer_immediate (i))
-	return error_mark_node;
-
     return r.resolve_to (r.mode_suffix_id, type);
   }
 
@@ -1097,7 +1105,7 @@  struct binary_rshift_narrow_def : public overloaded_base<0>
 	 bool preserve_user_namespace) const override
   {
     b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
-    build_all (b, "vh0,vh0,v0,ss32", group, MODE_n, preserve_user_namespace);
+    build_all (b, "vh0,vh0,v0,su64", group, MODE_n, preserve_user_namespace);
   }
 
   tree
@@ -1144,7 +1152,7 @@  struct binary_rshift_narrow_unsigned_def : public overloaded_base<0>
 	 bool preserve_user_namespace) const override
   {
     b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
-    build_all (b, "vhu0,vhu0,v0,ss32", group, MODE_n, preserve_user_namespace);
+    build_all (b, "vhu0,vhu0,v0,su64", group, MODE_n, preserve_user_namespace);
   }
 
   tree
@@ -1588,7 +1596,7 @@  struct ternary_lshift_def : public overloaded_base<0>
 	 bool preserve_user_namespace) const override
   {
     b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
-    build_all (b, "v0,v0,v0,ss32", group, MODE_n, preserve_user_namespace);
+    build_all (b, "v0,v0,v0,su64", group, MODE_n, preserve_user_namespace);
   }
 
   tree
@@ -1683,7 +1691,7 @@  struct ternary_rshift_def : public overloaded_base<0>
 	 bool preserve_user_namespace) const override
   {
     b.add_overloaded_functions (group, MODE_n, preserve_user_namespace);
-    build_all (b, "v0,v0,v0,ss32", group, MODE_n, preserve_user_namespace);
+    build_all (b, "v0,v0,v0,su64", group, MODE_n, preserve_user_namespace);
   }
 
   tree
@@ -1838,11 +1846,18 @@  struct unary_int32_acc_def : public overloaded_base<0>
   {
     unsigned int i, nargs;
     type_suffix_index type;
+    const char *first_type_name;
+
     if (!r.check_gp_argument (2, i, nargs)
-	|| !r.require_integer_immediate (0)
 	|| (type = r.infer_vector_type (1)) == NUM_TYPE_SUFFIXES)
       return error_mark_node;
 
+    first_type_name = (type_suffixes[type].unsigned_p
+		       ? "uint32_t"
+		       : "int32_t");
+    if (!r.require_scalar_type (0, first_type_name))
+      return error_mark_node;
+
     return r.resolve_to (r.mode_suffix_id, type);
   }
 };