Re: [PATCH 3/13] rs6000, fix error in unsigned vector float to unsigned int built-in definitions

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi,

on 2024/4/20 05:17, Carl Love wrote:
> rs6000, fix error in unsigned vector float to unsigned  int built-in definitions
> 
> The built-ins __builtin_vsx_vunsigned_v2df and__builtin_vsx_vunsigned_v4sf
> are supposed to take a vector of floats and return a vector of unsigned
> long long ints.  The definitions are using the signed version of the

Sorry for nitpicking, here __builtin_vsx_vunsigned_v2df takes vector of doubles
and returns vector of unsigned long long ints while __builtin_vsx_vunsigned_v4sf
takes vector of floats and returns vector of unsigned ints.

> instructions not the unsigned version of the instruction.  The results
> should also be unsigned.  The builtins are used by the overloaded
> vec_unsigned builtin which has an unsigned result.
> 
> Similarly the built-ins __builtin_vsx_vunsignede_v2df and
> __builtin_vsx_vunsignedo_v2df are supposed to retun an unsigned result.

Nit: s/retun/return/

> If the floating point argument is negative, the unsigned result is zero.
> The built-ins are used in the overloaded built-in vec_unsignede and
> vec_unsignedo respectively.
> 
> Add a test cases for a negative floating point arguments for each of the
> above built-ins.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-builtins.def (__builtin_vsx_vunsigned_v2df,
> 	__builtin_vsx_vunsigned_v4sf, __builtin_vsx_vunsignede_v2df,
> 	__builtin_vsx_vunsignedo_v2df): Change the result type to unsigned.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/powerpc/builtins-3-runnable.c: Add tests for
> 	vec_unsignede and vec_unsignedo with negative arguments.
> ---
>  gcc/config/rs6000/rs6000-builtins.def         | 12 +++++-----
>  .../gcc.target/powerpc/builtins-3-runnable.c  | 23 ++++++++++++++++---
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index c6d2ea1bc39..bf9a0ae22fc 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1580,16 +1580,16 @@
>    const vsi __builtin_vsx_vsignedo_v2df (vd);
>      VEC_VSIGNEDO_V2DF vsignedo_v2df {}
>  
> -  const vsll __builtin_vsx_vunsigned_v2df (vd);
> -    VEC_VUNSIGNED_V2DF vsx_xvcvdpsxds {}
> +  const vull __builtin_vsx_vunsigned_v2df (vd);
> +    VEC_VUNSIGNED_V2DF vsx_xvcvdpuxds {}
>  
> -  const vsi __builtin_vsx_vunsigned_v4sf (vf);
> -    VEC_VUNSIGNED_V4SF vsx_xvcvspsxws {}
> +  const vui __builtin_vsx_vunsigned_v4sf (vf);
> +    VEC_VUNSIGNED_V4SF vsx_xvcvspuxws {}
>  
> -  const vsi __builtin_vsx_vunsignede_v2df (vd);
> +  const vui __builtin_vsx_vunsignede_v2df (vd);
>      VEC_VUNSIGNEDE_V2DF vunsignede_v2df {}
>  
> -  const vsi __builtin_vsx_vunsignedo_v2df (vd);
> +  const vui __builtin_vsx_vunsignedo_v2df (vd);
>      VEC_VUNSIGNEDO_V2DF vunsignedo_v2df {}
>  
>    const vf __builtin_vsx_xscvdpsp (double);
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> index 0231a1fd086..6d4fe84c8a1 100644
> --- a/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> @@ -313,6 +313,15 @@ int main()
>  	test_unsigned_int_result (ALL, vec_uns_int_result,
>  				  vec_uns_int_expected);
>  
> +	/* Convert single precision float to  unsigned int.  Negative
> +	   arguments
> +	 */
> +	vec_flt0 = (vector float){-14.930, -834.49, -3.3, -5.4};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
> +	vec_uns_int_result = vec_unsigned (vec_flt0);
> +	test_unsigned_int_result (ALL, vec_uns_int_result,
> +				  vec_uns_int_expected);
> +
>  	/* Convert double precision float to long long unsigned int */
>  	vec_dble0 = (vector double){124.930, 8134.49};
>  	vec_ll_uns_int_expected = (vector long long unsigned int){124, 8134};
> @@ -321,9 +330,9 @@ int main()
>  				     vec_ll_uns_int_expected);

Nit: Similar coverage on negative for vector double can be added here.

BR,
Kewen

>  
>  	/* Convert double precision vector float to vector unsigned int,
> -	   even words */
> -	vec_dble0 = (vector double){3124.930, 8234.49};
> -	vec_uns_int_expected = (vector unsigned int){3124, 0, 8234, 0};
> +	   even words.  Negative arguments */
> +	vec_dble0 = (vector double){-124.930, -234.49};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
>  	vec_uns_int_result = vec_unsignede (vec_dble0);
>  	test_unsigned_int_result (EVEN, vec_uns_int_result,
>  				  vec_uns_int_expected);
> @@ -335,5 +344,13 @@ int main()
>  	vec_uns_int_result = vec_unsignedo (vec_dble0);
>  	test_unsigned_int_result (ODD, vec_uns_int_result,
>  				  vec_uns_int_expected);
> +
> +	/* Convert double precision vector float to vector unsigned int,
> +	   odd words.  Negative arguments.  */
> +	vec_dble0 = (vector double){-924.930, -1234.49};
> +	vec_uns_int_expected = (vector unsigned int){0, 0, 0, 0};
> +	vec_uns_int_result = vec_unsignedo (vec_dble0);
> +	test_unsigned_int_result (ODD, vec_uns_int_result,
> +				  vec_uns_int_expected);
>  }
>