c, c++: Add new value for vector types for __builtin_classify_type (type)

  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.


	Jakub
  

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
>
  

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).