public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c, c++: Add new value for vector types for __builtin_classify_type (type)
@ 2023-11-11  8:22 Jakub Jelinek
  2023-11-16 21:52 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-11-11  8:22 UTC (permalink / raw)
  To: Joseph S. Myers, Jason Merrill; +Cc: gcc-patches

Hi!

While filing a clang request to return 18 on _BitInts for
__builtin_classify_type instead of -1 they return currently, I've
noticed that we return -1 for vector types.  I'm not convinced it is a good
idea to change behavior of __builtin_classify_type (vector_expression)
after 22 years it behaved one way (returned -1), but the
__builtin_classify_type (type) form is a new extension added for GCC 14,
so this patch returns 19 for vectors just in that second form.  Many other
return values are only accessible from the second form as well (mostly because
of argument promotions), so I think it is fine like that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-11-11  Jakub Jelinek  <jakub@redhat.com>

gcc/
	* typeclass.h (enum type_class): Add vector_type_class.
	* builtins.h (type_to_class): Add FOR_TYPE argument.
	* builtins.cc (type_to_class): Add FOR_TYPE argument,
	for VECTOR_TYPE return vector_type_class if it is true, no_type_class
	otherwise.
	(expand_builtin_classify_type, fold_builtin_classify_type): Pass
	false to type_to_class second argument.
gcc/c/
	* c-parser.cc (c_parser_postfix_expression_after_primary): Pass true
	to type_to_class second argument.
gcc/cp/
	* parser.cc (cp_parser_postfix_expression): Pass true to type_to_class
	second argument.
	* pt.cc (tsubst_expr): Likewise.
gcc/testsuite/
	* c-c++-common/builtin-classify-type-1.c (main): Add tests for vector
	types.

--- gcc/typeclass.h.jj	2023-09-06 17:28:24.238977355 +0200
+++ gcc/typeclass.h	2023-11-10 10:50:59.519007647 +0100
@@ -38,7 +38,7 @@ enum type_class
   record_type_class, union_type_class,
   array_type_class, string_type_class,
   lang_type_class, opaque_type_class,
-  bitint_type_class
+  bitint_type_class, vector_type_class
 };
 
 #endif /* GCC_TYPECLASS_H */
--- gcc/builtins.h.jj	2023-09-29 10:39:37.073836032 +0200
+++ gcc/builtins.h	2023-11-10 11:17:22.196907216 +0100
@@ -156,6 +156,6 @@ extern internal_fn associated_internal_f
 extern internal_fn replacement_internal_fn (gcall *);
 
 extern bool builtin_with_linkage_p (tree);
-extern int type_to_class (tree);
+extern int type_to_class (tree, bool);
 
 #endif /* GCC_BUILTINS_H */
--- gcc/builtins.cc.jj	2023-11-09 09:17:40.230182483 +0100
+++ gcc/builtins.cc	2023-11-10 11:19:29.669129855 +0100
@@ -1833,10 +1833,11 @@ expand_builtin_return (rtx result)
   expand_naked_return ();
 }
 
-/* Used by expand_builtin_classify_type and fold_builtin_classify_type.  */
+/* Used by expand_builtin_classify_type and fold_builtin_classify_type.
+   FOR_TYPE is true for __builtin_classify_type (type), false otherwise.  */
 
 int
-type_to_class (tree type)
+type_to_class (tree type, bool for_type)
 {
   switch (TREE_CODE (type))
     {
@@ -1859,6 +1860,7 @@ type_to_class (tree type)
     case LANG_TYPE:	   return lang_type_class;
     case OPAQUE_TYPE:      return opaque_type_class;
     case BITINT_TYPE:	   return bitint_type_class;
+    case VECTOR_TYPE:	   return for_type ? vector_type_class : no_type_class;
     default:		   return no_type_class;
     }
 }
@@ -1869,7 +1871,7 @@ static rtx
 expand_builtin_classify_type (tree exp)
 {
   if (call_expr_nargs (exp))
-    return GEN_INT (type_to_class (TREE_TYPE (CALL_EXPR_ARG (exp, 0))));
+    return GEN_INT (type_to_class (TREE_TYPE (CALL_EXPR_ARG (exp, 0)), false));
   return GEN_INT (no_type_class);
 }
 
@@ -8678,7 +8680,8 @@ fold_builtin_classify_type (tree arg)
   if (arg == 0)
     return build_int_cst (integer_type_node, no_type_class);
 
-  return build_int_cst (integer_type_node, type_to_class (TREE_TYPE (arg)));
+  return build_int_cst (integer_type_node,
+			type_to_class (TREE_TYPE (arg), false));
 }
 
 /* Fold a call EXPR (which may be null) to __builtin_strlen with argument
--- gcc/c/c-parser.cc.jj	2023-11-09 09:04:18.473545429 +0100
+++ gcc/c/c-parser.cc	2023-11-10 11:19:57.907735925 +0100
@@ -12249,7 +12249,7 @@ c_parser_postfix_expression_after_primar
 					   &ret.expr_const_operands);
 		parens.skip_until_found_close (parser);
 		expr.value = build_int_cst (integer_type_node,
-					    type_to_class (ret.spec));
+					    type_to_class (ret.spec, true));
 		break;
 	      }
 	    else
--- gcc/cp/parser.cc.jj	2023-11-09 09:04:18.771541207 +0100
+++ gcc/cp/parser.cc	2023-11-10 11:20:24.732360518 +0100
@@ -7974,11 +7974,9 @@ cp_parser_postfix_expression (cp_parser
 			TREE_TYPE (postfix_expression) = integer_type_node;
 		      }
 		    else
-		      {
-			postfix_expression
-			  = build_int_cst (integer_type_node,
-					   type_to_class (type));
-		      }
+		      postfix_expression
+			= build_int_cst (integer_type_node,
+					 type_to_class (type, true));
 		    break;
 		  }
 	      }
--- gcc/cp/pt.cc.jj	2023-11-04 09:02:35.456000422 +0100
+++ gcc/cp/pt.cc	2023-11-10 11:20:42.783107902 +0100
@@ -20434,7 +20434,8 @@ tsubst_expr (tree t, tree args, tsubst_f
 		TREE_TYPE (ret) = integer_type_node;
 	      }
 	    else
-	      ret = build_int_cst (integer_type_node, type_to_class (type));
+	      ret = build_int_cst (integer_type_node,
+				   type_to_class (type, true));
 	    RETURN (ret);
 	  }
 	else if (koenig_p
--- gcc/testsuite/c-c++-common/builtin-classify-type-1.c.jj	2023-09-26 09:25:30.019599039 +0200
+++ gcc/testsuite/c-c++-common/builtin-classify-type-1.c	2023-11-10 11:02:01.927776922 +0100
@@ -22,6 +22,10 @@ main ()
   const char *p = (const char *) 0;
   float f = 0.0;
   _Complex double c = 0.0;
+  typedef int VI __attribute__((vector_size (4 * sizeof (int))));
+  typedef float VF __attribute__((vector_size (4 * sizeof (int))));
+  VI vi = { 0, 0, 0, 0 };
+  VF vf = { 0.0f, 0.0f, 0.0f, 0.0f };
 #ifdef __cplusplus
   struct T { void foo (); };
   int &r = a[0];
@@ -43,6 +47,8 @@ main ()
   static_assert (__builtin_classify_type (struct S) == 12, "");
   static_assert (__builtin_classify_type (union U) == 13, "");
   static_assert (__builtin_classify_type (int [2]) == 14, "");
+  static_assert (__builtin_classify_type (VI) == 19, "");
+  static_assert (__builtin_classify_type (VF) == 19, "");
   static_assert (__builtin_classify_type (__typeof__ (a[0])) == 1, "");
   static_assert (__builtin_classify_type (__typeof__ (e)) == 3, "");
   static_assert (__builtin_classify_type (__typeof__ (b)) == 4, "");
@@ -57,6 +63,8 @@ main ()
   static_assert (__builtin_classify_type (__typeof__ (s)) == 12, "");
   static_assert (__builtin_classify_type (__typeof__ (u)) == 13, "");
   static_assert (__builtin_classify_type (__typeof__ (a)) == 14, "");
+  static_assert (__builtin_classify_type (__typeof__ (vi)) == 19, "");
+  static_assert (__builtin_classify_type (__typeof__ (vf)) == 19, "");
 #ifndef __cplusplus
   static_assert (__builtin_classify_type (a[0]) == 1, "");
   static_assert (__builtin_classify_type (e) == 1, "");
@@ -102,4 +110,8 @@ main ()
     abort ();
   if (__builtin_classify_type (a) != 5)
     abort ();
+  if (__builtin_classify_type (vi) != -1)
+    abort ();
+  if (__builtin_classify_type (vf) != -1)
+    abort ();
 }

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c, c++: Add new value for vector types for __builtin_classify_type (type)
  2023-11-11  8:22 [PATCH] c, c++: Add new value for vector types for __builtin_classify_type (type) Jakub Jelinek
@ 2023-11-16 21:52 ` Jason Merrill
  2023-11-16 23:49   ` Joseph Myers
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2023-11-16 21:52 UTC (permalink / raw)
  To: Jakub Jelinek, Joseph S. Myers; +Cc: gcc-patches

On 11/11/23 03:22, Jakub Jelinek wrote:
> Hi!
> 
> While filing a clang request to return 18 on _BitInts for
> __builtin_classify_type instead of -1 they return currently, I've
> noticed that we return -1 for vector types.  I'm not convinced it is a good
> idea to change behavior of __builtin_classify_type (vector_expression)
> after 22 years it behaved one way (returned -1), but the
> __builtin_classify_type (type) form is a new extension added for GCC 14,
> so this patch returns 19 for vectors just in that second form.  Many other
> return values are only accessible from the second form as well (mostly because
> of argument promotions), so I think it is fine like that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

The C++ changes are OK (and obvious).  I'm skeptical of the choice to 
keep returning -1 for the expression form, it seems more likely to cause 
problems (due to it disagreeing with the type form) than changing it 
(due to old code somehow relying on -1?).  But people who are more 
familiar with the use of __builtin_classify_type should make the call.

> 2023-11-11  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/
> 	* typeclass.h (enum type_class): Add vector_type_class.
> 	* builtins.h (type_to_class): Add FOR_TYPE argument.
> 	* builtins.cc (type_to_class): Add FOR_TYPE argument,
> 	for VECTOR_TYPE return vector_type_class if it is true, no_type_class
> 	otherwise.
> 	(expand_builtin_classify_type, fold_builtin_classify_type): Pass
> 	false to type_to_class second argument.
> gcc/c/
> 	* c-parser.cc (c_parser_postfix_expression_after_primary): Pass true
> 	to type_to_class second argument.
> gcc/cp/
> 	* parser.cc (cp_parser_postfix_expression): Pass true to type_to_class
> 	second argument.
> 	* pt.cc (tsubst_expr): Likewise.
> gcc/testsuite/
> 	* c-c++-common/builtin-classify-type-1.c (main): Add tests for vector
> 	types.
> 
> --- gcc/typeclass.h.jj	2023-09-06 17:28:24.238977355 +0200
> +++ gcc/typeclass.h	2023-11-10 10:50:59.519007647 +0100
> @@ -38,7 +38,7 @@ enum type_class
>     record_type_class, union_type_class,
>     array_type_class, string_type_class,
>     lang_type_class, opaque_type_class,
> -  bitint_type_class
> +  bitint_type_class, vector_type_class
>   };
>   
>   #endif /* GCC_TYPECLASS_H */
> --- gcc/builtins.h.jj	2023-09-29 10:39:37.073836032 +0200
> +++ gcc/builtins.h	2023-11-10 11:17:22.196907216 +0100
> @@ -156,6 +156,6 @@ extern internal_fn associated_internal_f
>   extern internal_fn replacement_internal_fn (gcall *);
>   
>   extern bool builtin_with_linkage_p (tree);
> -extern int type_to_class (tree);
> +extern int type_to_class (tree, bool);
>   
>   #endif /* GCC_BUILTINS_H */
> --- gcc/builtins.cc.jj	2023-11-09 09:17:40.230182483 +0100
> +++ gcc/builtins.cc	2023-11-10 11:19:29.669129855 +0100
> @@ -1833,10 +1833,11 @@ expand_builtin_return (rtx result)
>     expand_naked_return ();
>   }
>   
> -/* Used by expand_builtin_classify_type and fold_builtin_classify_type.  */
> +/* Used by expand_builtin_classify_type and fold_builtin_classify_type.
> +   FOR_TYPE is true for __builtin_classify_type (type), false otherwise.  */
>   
>   int
> -type_to_class (tree type)
> +type_to_class (tree type, bool for_type)
>   {
>     switch (TREE_CODE (type))
>       {
> @@ -1859,6 +1860,7 @@ type_to_class (tree type)
>       case LANG_TYPE:	   return lang_type_class;
>       case OPAQUE_TYPE:      return opaque_type_class;
>       case BITINT_TYPE:	   return bitint_type_class;
> +    case VECTOR_TYPE:	   return for_type ? vector_type_class : no_type_class;
>       default:		   return no_type_class;
>       }
>   }
> @@ -1869,7 +1871,7 @@ static rtx
>   expand_builtin_classify_type (tree exp)
>   {
>     if (call_expr_nargs (exp))
> -    return GEN_INT (type_to_class (TREE_TYPE (CALL_EXPR_ARG (exp, 0))));
> +    return GEN_INT (type_to_class (TREE_TYPE (CALL_EXPR_ARG (exp, 0)), false));
>     return GEN_INT (no_type_class);
>   }
>   
> @@ -8678,7 +8680,8 @@ fold_builtin_classify_type (tree arg)
>     if (arg == 0)
>       return build_int_cst (integer_type_node, no_type_class);
>   
> -  return build_int_cst (integer_type_node, type_to_class (TREE_TYPE (arg)));
> +  return build_int_cst (integer_type_node,
> +			type_to_class (TREE_TYPE (arg), false));
>   }
>   
>   /* Fold a call EXPR (which may be null) to __builtin_strlen with argument
> --- gcc/c/c-parser.cc.jj	2023-11-09 09:04:18.473545429 +0100
> +++ gcc/c/c-parser.cc	2023-11-10 11:19:57.907735925 +0100
> @@ -12249,7 +12249,7 @@ c_parser_postfix_expression_after_primar
>   					   &ret.expr_const_operands);
>   		parens.skip_until_found_close (parser);
>   		expr.value = build_int_cst (integer_type_node,
> -					    type_to_class (ret.spec));
> +					    type_to_class (ret.spec, true));
>   		break;
>   	      }
>   	    else
> --- gcc/cp/parser.cc.jj	2023-11-09 09:04:18.771541207 +0100
> +++ gcc/cp/parser.cc	2023-11-10 11:20:24.732360518 +0100
> @@ -7974,11 +7974,9 @@ cp_parser_postfix_expression (cp_parser
>   			TREE_TYPE (postfix_expression) = integer_type_node;
>   		      }
>   		    else
> -		      {
> -			postfix_expression
> -			  = build_int_cst (integer_type_node,
> -					   type_to_class (type));
> -		      }
> +		      postfix_expression
> +			= build_int_cst (integer_type_node,
> +					 type_to_class (type, true));
>   		    break;
>   		  }
>   	      }
> --- gcc/cp/pt.cc.jj	2023-11-04 09:02:35.456000422 +0100
> +++ gcc/cp/pt.cc	2023-11-10 11:20:42.783107902 +0100
> @@ -20434,7 +20434,8 @@ tsubst_expr (tree t, tree args, tsubst_f
>   		TREE_TYPE (ret) = integer_type_node;
>   	      }
>   	    else
> -	      ret = build_int_cst (integer_type_node, type_to_class (type));
> +	      ret = build_int_cst (integer_type_node,
> +				   type_to_class (type, true));
>   	    RETURN (ret);
>   	  }
>   	else if (koenig_p
> --- gcc/testsuite/c-c++-common/builtin-classify-type-1.c.jj	2023-09-26 09:25:30.019599039 +0200
> +++ gcc/testsuite/c-c++-common/builtin-classify-type-1.c	2023-11-10 11:02:01.927776922 +0100
> @@ -22,6 +22,10 @@ main ()
>     const char *p = (const char *) 0;
>     float f = 0.0;
>     _Complex double c = 0.0;
> +  typedef int VI __attribute__((vector_size (4 * sizeof (int))));
> +  typedef float VF __attribute__((vector_size (4 * sizeof (int))));
> +  VI vi = { 0, 0, 0, 0 };
> +  VF vf = { 0.0f, 0.0f, 0.0f, 0.0f };
>   #ifdef __cplusplus
>     struct T { void foo (); };
>     int &r = a[0];
> @@ -43,6 +47,8 @@ main ()
>     static_assert (__builtin_classify_type (struct S) == 12, "");
>     static_assert (__builtin_classify_type (union U) == 13, "");
>     static_assert (__builtin_classify_type (int [2]) == 14, "");
> +  static_assert (__builtin_classify_type (VI) == 19, "");
> +  static_assert (__builtin_classify_type (VF) == 19, "");
>     static_assert (__builtin_classify_type (__typeof__ (a[0])) == 1, "");
>     static_assert (__builtin_classify_type (__typeof__ (e)) == 3, "");
>     static_assert (__builtin_classify_type (__typeof__ (b)) == 4, "");
> @@ -57,6 +63,8 @@ main ()
>     static_assert (__builtin_classify_type (__typeof__ (s)) == 12, "");
>     static_assert (__builtin_classify_type (__typeof__ (u)) == 13, "");
>     static_assert (__builtin_classify_type (__typeof__ (a)) == 14, "");
> +  static_assert (__builtin_classify_type (__typeof__ (vi)) == 19, "");
> +  static_assert (__builtin_classify_type (__typeof__ (vf)) == 19, "");
>   #ifndef __cplusplus
>     static_assert (__builtin_classify_type (a[0]) == 1, "");
>     static_assert (__builtin_classify_type (e) == 1, "");
> @@ -102,4 +110,8 @@ main ()
>       abort ();
>     if (__builtin_classify_type (a) != 5)
>       abort ();
> +  if (__builtin_classify_type (vi) != -1)
> +    abort ();
> +  if (__builtin_classify_type (vf) != -1)
> +    abort ();
>   }
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c, c++: Add new value for vector types for __builtin_classify_type (type)
  2023-11-16 21:52 ` Jason Merrill
@ 2023-11-16 23:49   ` Joseph Myers
  2023-11-17 14:04     ` [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2023-11-16 23:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Jakub Jelinek, gcc-patches

On Thu, 16 Nov 2023, Jason Merrill wrote:

> On 11/11/23 03:22, Jakub Jelinek wrote:
> > Hi!
> > 
> > While filing a clang request to return 18 on _BitInts for
> > __builtin_classify_type instead of -1 they return currently, I've
> > noticed that we return -1 for vector types.  I'm not convinced it is a good
> > idea to change behavior of __builtin_classify_type (vector_expression)
> > after 22 years it behaved one way (returned -1), but the
> > __builtin_classify_type (type) form is a new extension added for GCC 14,
> > so this patch returns 19 for vectors just in that second form.  Many other
> > return values are only accessible from the second form as well (mostly
> > because
> > of argument promotions), so I think it is fine like that.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> The C++ changes are OK (and obvious).  I'm skeptical of the choice to keep
> returning -1 for the expression form, it seems more likely to cause problems
> (due to it disagreeing with the type form) than changing it (due to old code
> somehow relying on -1?).  But people who are more familiar with the use of
> __builtin_classify_type should make the call.

I'm also doubtful of keeping returning -1 for vectors in expression form 
(I'd be surprised if people are actually using __builtin_classify_type 
with vectors).  The C changes are OK (but the front-end changes wouldn't 
be needed at all if the vector and type argument cases aren't 
distinguished).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type
  2023-11-16 23:49   ` Joseph Myers
@ 2023-11-17 14:04     ` Jakub Jelinek
  2023-11-20  8:03       ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-11-17 14:04 UTC (permalink / raw)
  To: Joseph Myers, Jason Merrill, Richard Biener; +Cc: gcc-patches

On Thu, Nov 16, 2023 at 11:49:03PM +0000, Joseph Myers wrote:
> > > While filing a clang request to return 18 on _BitInts for
> > > __builtin_classify_type instead of -1 they return currently, I've
> > > noticed that we return -1 for vector types.  I'm not convinced it is a good
> > > idea to change behavior of __builtin_classify_type (vector_expression)
> > > after 22 years it behaved one way (returned -1), but the
> > > __builtin_classify_type (type) form is a new extension added for GCC 14,
> > > so this patch returns 19 for vectors just in that second form.  Many other
> > > return values are only accessible from the second form as well (mostly
> > > because
> > > of argument promotions), so I think it is fine like that.
> > > 
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > The C++ changes are OK (and obvious).  I'm skeptical of the choice to keep
> > returning -1 for the expression form, it seems more likely to cause problems
> > (due to it disagreeing with the type form) than changing it (due to old code
> > somehow relying on -1?).  But people who are more familiar with the use of
> > __builtin_classify_type should make the call.
> 
> I'm also doubtful of keeping returning -1 for vectors in expression form 
> (I'd be surprised if people are actually using __builtin_classify_type 
> with vectors).  The C changes are OK (but the front-end changes wouldn't 
> be needed at all if the vector and type argument cases aren't 
> distinguished).

Given that you've both agreed that you think changing
__builtin_classify_type (vector_expr) is ok (hopefully in the wild people
test bct against known values rather than pretending to know what the
unknown -1 means because it can mean multiple type kinds), here is a simpler
patch which changes it even for that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-11-17  Jakub Jelinek  <jakub@redhat.com>

gcc/
	* typeclass.h (enum type_class): Add vector_type_class.
	* builtins.cc (type_to_class): Return vector_type_class for
	VECTOR_TYPE.
gcc/testsuite/
	* c-c++-common/builtin-classify-type-1.c (main): Add tests for vector
	types.

--- gcc/typeclass.h.jj	2023-09-06 17:28:24.238977355 +0200
+++ gcc/typeclass.h	2023-11-10 10:50:59.519007647 +0100
@@ -38,7 +38,7 @@ enum type_class
   record_type_class, union_type_class,
   array_type_class, string_type_class,
   lang_type_class, opaque_type_class,
-  bitint_type_class
+  bitint_type_class, vector_type_class
 };
 
 #endif /* GCC_TYPECLASS_H */
--- gcc/builtins.cc.jj	2023-11-09 09:17:40.230182483 +0100
+++ gcc/builtins.cc	2023-11-10 11:19:29.669129855 +0100
@@ -1859,6 +1859,7 @@ type_to_class (tree type)
     case LANG_TYPE:	   return lang_type_class;
     case OPAQUE_TYPE:      return opaque_type_class;
     case BITINT_TYPE:	   return bitint_type_class;
+    case VECTOR_TYPE:	   return vector_type_class;
     default:		   return no_type_class;
     }
 }
--- gcc/testsuite/c-c++-common/builtin-classify-type-1.c.jj	2023-09-26 09:25:30.019599039 +0200
+++ gcc/testsuite/c-c++-common/builtin-classify-type-1.c	2023-11-10 11:02:01.927776922 +0100
@@ -22,6 +22,10 @@ main ()
   const char *p = (const char *) 0;
   float f = 0.0;
   _Complex double c = 0.0;
+  typedef int VI __attribute__((vector_size (4 * sizeof (int))));
+  typedef float VF __attribute__((vector_size (4 * sizeof (int))));
+  VI vi = { 0, 0, 0, 0 };
+  VF vf = { 0.0f, 0.0f, 0.0f, 0.0f };
 #ifdef __cplusplus
   struct T { void foo (); };
   int &r = a[0];
@@ -43,6 +47,8 @@ main ()
   static_assert (__builtin_classify_type (struct S) == 12, "");
   static_assert (__builtin_classify_type (union U) == 13, "");
   static_assert (__builtin_classify_type (int [2]) == 14, "");
+  static_assert (__builtin_classify_type (VI) == 19, "");
+  static_assert (__builtin_classify_type (VF) == 19, "");
   static_assert (__builtin_classify_type (__typeof__ (a[0])) == 1, "");
   static_assert (__builtin_classify_type (__typeof__ (e)) == 3, "");
   static_assert (__builtin_classify_type (__typeof__ (b)) == 4, "");
@@ -57,6 +63,8 @@ main ()
   static_assert (__builtin_classify_type (__typeof__ (s)) == 12, "");
   static_assert (__builtin_classify_type (__typeof__ (u)) == 13, "");
   static_assert (__builtin_classify_type (__typeof__ (a)) == 14, "");
+  static_assert (__builtin_classify_type (__typeof__ (vi)) == 19, "");
+  static_assert (__builtin_classify_type (__typeof__ (vf)) == 19, "");
 #ifndef __cplusplus
   static_assert (__builtin_classify_type (a[0]) == 1, "");
   static_assert (__builtin_classify_type (e) == 1, "");
@@ -102,4 +110,8 @@ main ()
     abort ();
   if (__builtin_classify_type (a) != 5)
     abort ();
+  if (__builtin_classify_type (vi) != 19)
+    abort ();
+  if (__builtin_classify_type (vf) != 19)
+    abort ();
 }


	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type
  2023-11-17 14:04     ` [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type Jakub Jelinek
@ 2023-11-20  8:03       ` Richard Biener
  2023-11-20  9:41         ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2023-11-20  8:03 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Joseph Myers, Jason Merrill, gcc-patches

On Fri, 17 Nov 2023, Jakub Jelinek wrote:

> On Thu, Nov 16, 2023 at 11:49:03PM +0000, Joseph Myers wrote:
> > > > While filing a clang request to return 18 on _BitInts for
> > > > __builtin_classify_type instead of -1 they return currently, I've
> > > > noticed that we return -1 for vector types.  I'm not convinced it is a good
> > > > idea to change behavior of __builtin_classify_type (vector_expression)
> > > > after 22 years it behaved one way (returned -1), but the
> > > > __builtin_classify_type (type) form is a new extension added for GCC 14,
> > > > so this patch returns 19 for vectors just in that second form.  Many other
> > > > return values are only accessible from the second form as well (mostly
> > > > because
> > > > of argument promotions), so I think it is fine like that.
> > > > 
> > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > 
> > > The C++ changes are OK (and obvious).  I'm skeptical of the choice to keep
> > > returning -1 for the expression form, it seems more likely to cause problems
> > > (due to it disagreeing with the type form) than changing it (due to old code
> > > somehow relying on -1?).  But people who are more familiar with the use of
> > > __builtin_classify_type should make the call.
> > 
> > I'm also doubtful of keeping returning -1 for vectors in expression form 
> > (I'd be surprised if people are actually using __builtin_classify_type 
> > with vectors).  The C changes are OK (but the front-end changes wouldn't 
> > be needed at all if the vector and type argument cases aren't 
> > distinguished).
> 
> Given that you've both agreed that you think changing
> __builtin_classify_type (vector_expr) is ok (hopefully in the wild people
> test bct against known values rather than pretending to know what the
> unknown -1 means because it can mean multiple type kinds), here is a simpler
> patch which changes it even for that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.  Do we have to adjust any of our documentation for this?

> 2023-11-17  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/
> 	* typeclass.h (enum type_class): Add vector_type_class.
> 	* builtins.cc (type_to_class): Return vector_type_class for
> 	VECTOR_TYPE.
> gcc/testsuite/
> 	* c-c++-common/builtin-classify-type-1.c (main): Add tests for vector
> 	types.
> 
> --- gcc/typeclass.h.jj	2023-09-06 17:28:24.238977355 +0200
> +++ gcc/typeclass.h	2023-11-10 10:50:59.519007647 +0100
> @@ -38,7 +38,7 @@ enum type_class
>    record_type_class, union_type_class,
>    array_type_class, string_type_class,
>    lang_type_class, opaque_type_class,
> -  bitint_type_class
> +  bitint_type_class, vector_type_class
>  };
>  
>  #endif /* GCC_TYPECLASS_H */
> --- gcc/builtins.cc.jj	2023-11-09 09:17:40.230182483 +0100
> +++ gcc/builtins.cc	2023-11-10 11:19:29.669129855 +0100
> @@ -1859,6 +1859,7 @@ type_to_class (tree type)
>      case LANG_TYPE:	   return lang_type_class;
>      case OPAQUE_TYPE:      return opaque_type_class;
>      case BITINT_TYPE:	   return bitint_type_class;
> +    case VECTOR_TYPE:	   return vector_type_class;
>      default:		   return no_type_class;
>      }
>  }
> --- gcc/testsuite/c-c++-common/builtin-classify-type-1.c.jj	2023-09-26 09:25:30.019599039 +0200
> +++ gcc/testsuite/c-c++-common/builtin-classify-type-1.c	2023-11-10 11:02:01.927776922 +0100
> @@ -22,6 +22,10 @@ main ()
>    const char *p = (const char *) 0;
>    float f = 0.0;
>    _Complex double c = 0.0;
> +  typedef int VI __attribute__((vector_size (4 * sizeof (int))));
> +  typedef float VF __attribute__((vector_size (4 * sizeof (int))));
> +  VI vi = { 0, 0, 0, 0 };
> +  VF vf = { 0.0f, 0.0f, 0.0f, 0.0f };
>  #ifdef __cplusplus
>    struct T { void foo (); };
>    int &r = a[0];
> @@ -43,6 +47,8 @@ main ()
>    static_assert (__builtin_classify_type (struct S) == 12, "");
>    static_assert (__builtin_classify_type (union U) == 13, "");
>    static_assert (__builtin_classify_type (int [2]) == 14, "");
> +  static_assert (__builtin_classify_type (VI) == 19, "");
> +  static_assert (__builtin_classify_type (VF) == 19, "");
>    static_assert (__builtin_classify_type (__typeof__ (a[0])) == 1, "");
>    static_assert (__builtin_classify_type (__typeof__ (e)) == 3, "");
>    static_assert (__builtin_classify_type (__typeof__ (b)) == 4, "");
> @@ -57,6 +63,8 @@ main ()
>    static_assert (__builtin_classify_type (__typeof__ (s)) == 12, "");
>    static_assert (__builtin_classify_type (__typeof__ (u)) == 13, "");
>    static_assert (__builtin_classify_type (__typeof__ (a)) == 14, "");
> +  static_assert (__builtin_classify_type (__typeof__ (vi)) == 19, "");
> +  static_assert (__builtin_classify_type (__typeof__ (vf)) == 19, "");
>  #ifndef __cplusplus
>    static_assert (__builtin_classify_type (a[0]) == 1, "");
>    static_assert (__builtin_classify_type (e) == 1, "");
> @@ -102,4 +110,8 @@ main ()
>      abort ();
>    if (__builtin_classify_type (a) != 5)
>      abort ();
> +  if (__builtin_classify_type (vi) != 19)
> +    abort ();
> +  if (__builtin_classify_type (vf) != 19)
> +    abort ();
>  }
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type
  2023-11-20  8:03       ` Richard Biener
@ 2023-11-20  9:41         ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2023-11-20  9:41 UTC (permalink / raw)
  To: Richard Biener; +Cc: Joseph Myers, Jason Merrill, gcc-patches

On Mon, Nov 20, 2023 at 08:03:18AM +0000, Richard Biener wrote:
> OK.  Do we have to adjust any of our documentation for this?

I've done it this way.  We don't really document the exact values
in the documentation, so I think it is sufficient like that.
Committed to trunk.

2023-11-20  Jakub Jelinek  <jakub@redhat.com>

gcc/
	* typeclass.h (enum type_class): Add vector_type_class.
	* builtins.cc (type_to_class): Return vector_type_class for
	VECTOR_TYPE.
	* doc/extend.texi (__builtin_classify_type): Mention bit-precise
	integer types and vector types.
gcc/testsuite/
	* c-c++-common/builtin-classify-type-1.c (main): Add tests for vector
	types.

--- gcc/typeclass.h.jj	2023-09-06 17:28:24.238977355 +0200
+++ gcc/typeclass.h	2023-11-10 10:50:59.519007647 +0100
@@ -38,7 +38,7 @@ enum type_class
   record_type_class, union_type_class,
   array_type_class, string_type_class,
   lang_type_class, opaque_type_class,
-  bitint_type_class
+  bitint_type_class, vector_type_class
 };
 
 #endif /* GCC_TYPECLASS_H */
--- gcc/builtins.cc.jj	2023-11-09 09:17:40.230182483 +0100
+++ gcc/builtins.cc	2023-11-10 11:19:29.669129855 +0100
@@ -1859,6 +1859,7 @@ type_to_class (tree type)
     case LANG_TYPE:	   return lang_type_class;
     case OPAQUE_TYPE:      return opaque_type_class;
     case BITINT_TYPE:	   return bitint_type_class;
+    case VECTOR_TYPE:	   return vector_type_class;
     default:		   return no_type_class;
     }
 }
--- gcc/doc/extend.texi.jj
+++ gcc/doc/extend.texi
@@ -14746,11 +14746,11 @@ The @code{__builtin_classify_type} returns a small integer with a category
 of @var{arg} argument's type, like void type, integer type, enumeral type,
 boolean type, pointer type, reference type, offset type, real type, complex
 type, function type, method type, record type, union type, array type,
-string type, etc.  When the argument is an expression, for
-backwards compatibility reason the argument is promoted like arguments
-passed to @code{...} in varargs function, so some classes are never returned
-in certain languages.  Alternatively, the argument of the built-in
-function can be a typename, such as the @code{typeof} specifier.
+string type, bit-precise integer type, vector type, etc.  When the argument
+is an expression, for backwards compatibility reason the argument is promoted
+like arguments passed to @code{...} in varargs function, so some classes are
+never returned in certain languages.  Alternatively, the argument of the
+built-in function can be a typename, such as the @code{typeof} specifier.
 
 @smallexample
 int a[2];
--- gcc/testsuite/c-c++-common/builtin-classify-type-1.c.jj	2023-09-26 09:25:30.019599039 +0200
+++ gcc/testsuite/c-c++-common/builtin-classify-type-1.c	2023-11-10 11:02:01.927776922 +0100
@@ -22,6 +22,10 @@ main ()
   const char *p = (const char *) 0;
   float f = 0.0;
   _Complex double c = 0.0;
+  typedef int VI __attribute__((vector_size (4 * sizeof (int))));
+  typedef float VF __attribute__((vector_size (4 * sizeof (int))));
+  VI vi = { 0, 0, 0, 0 };
+  VF vf = { 0.0f, 0.0f, 0.0f, 0.0f };
 #ifdef __cplusplus
   struct T { void foo (); };
   int &r = a[0];
@@ -43,6 +47,8 @@ main ()
   static_assert (__builtin_classify_type (struct S) == 12, "");
   static_assert (__builtin_classify_type (union U) == 13, "");
   static_assert (__builtin_classify_type (int [2]) == 14, "");
+  static_assert (__builtin_classify_type (VI) == 19, "");
+  static_assert (__builtin_classify_type (VF) == 19, "");
   static_assert (__builtin_classify_type (__typeof__ (a[0])) == 1, "");
   static_assert (__builtin_classify_type (__typeof__ (e)) == 3, "");
   static_assert (__builtin_classify_type (__typeof__ (b)) == 4, "");
@@ -57,6 +63,8 @@ main ()
   static_assert (__builtin_classify_type (__typeof__ (s)) == 12, "");
   static_assert (__builtin_classify_type (__typeof__ (u)) == 13, "");
   static_assert (__builtin_classify_type (__typeof__ (a)) == 14, "");
+  static_assert (__builtin_classify_type (__typeof__ (vi)) == 19, "");
+  static_assert (__builtin_classify_type (__typeof__ (vf)) == 19, "");
 #ifndef __cplusplus
   static_assert (__builtin_classify_type (a[0]) == 1, "");
   static_assert (__builtin_classify_type (e) == 1, "");
@@ -102,4 +110,8 @@ main ()
     abort ();
   if (__builtin_classify_type (a) != 5)
     abort ();
+  if (__builtin_classify_type (vi) != 19)
+    abort ();
+  if (__builtin_classify_type (vf) != 19)
+    abort ();
 }


	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-11-21  8:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-11  8:22 [PATCH] c, c++: Add new value for vector types for __builtin_classify_type (type) Jakub Jelinek
2023-11-16 21:52 ` Jason Merrill
2023-11-16 23:49   ` Joseph Myers
2023-11-17 14:04     ` [PATCH] middle-end, v2: Add new value for vector types for __builtin_classify_type Jakub Jelinek
2023-11-20  8:03       ` Richard Biener
2023-11-20  9:41         ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).