Hi Carl,

on 2024/6/14 03:40, Carl Love wrote:
> 
> GCC maintainers:
> 
> The patch has been updated per the comments from version 3.  Please let me 
> know if the patch is acceptable for mainline.
> 
> Thanks.
> 
>                              Carl 
> 
> ---------------------------------------------------------
> 
> rs6000, extend vec_xxpermdi built-in for __int128 args
> 
> Add a new signed and unsigned overloaded instances for vec_xxpermdi
> 
>    __int128 vec_xxpermdi (__int128, __int128, const int);
>    __uint128 vec_xxpermdi (__uint128, __uint128, const int);

Nit: I think we need the "vector" keyword here to avoid confusion.

> 
> Update the documentation to include a reference to the new built-in
> instances.
> 
> Add test cases for the new overloaded instances.
> 
> gcc/ChangeLog:
>       * config/rs6000/rs6000-overload.def (vec_xxpermdi): Add new
>       overloaded built-in instances.

Better to mention something like:  "built-in instances for vector
signed and unsigned int128".

>       * doc/extend.texi:  Add documentation for new overloaded built-in

Nit: One more space before "Add".

>       instances.

... can be extended similarly.

> 
> gcc/testsuite/ChangeLog:gcc/testsuite/ChangeLog:
>       * gcc.target/powerpc/vec_perm-runnable-i128.c: New test file.
> ---
>  gcc/config/rs6000/rs6000-overload.def         |   4 +
>  gcc/doc/extend.texi                           |   4 +
>  .../powerpc/vec_perm-runnable-i128.c          | 229 ++++++++++++++++++
>  3 files changed, 237 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vec_perm-runnable-i128.c
> 
> diff --git a/gcc/config/rs6000/rs6000-overload.def 
> b/gcc/config/rs6000/rs6000-overload.def
> index 6cec1ad4f1a..354f8fabe0f 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -4936,6 +4936,10 @@
>      XXPERMDI_2DI  XXPERMDI_VSLL
>    vull __builtin_vsx_xxpermdi (vull, vull, const int);
>      XXPERMDI_2DI  XXPERMDI_VULL
> +  vsq __builtin_vsx_xxpermdi (vsq, vsq, const int);
> +    XXPERMDI_1TI  XXPERMDI_1SQ
> +  vuq __builtin_vsx_xxpermdi (vuq, vuq, const int);
> +    XXPERMDI_1TI  XXPERMDI_1UQ

Nit: XXPERMDI_1SQ -> XXPERMDI_SQ
     XXPERMDI_1UQ -> XXPERMDI_UQ
(removing "1" to align with the above).

>    vf __builtin_vsx_xxpermdi (vf, vf, const int);
>      XXPERMDI_4SF  XXPERMDI_VF
>    vd __builtin_vsx_xxpermdi (vd, vd, const int);
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index d7d8d149a43..9e45976436b 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -22610,6 +22610,10 @@ void vec_vsx_st (vector bool char, int, signed char 
> *);
>  
>  vector double vec_xxpermdi (vector double, vector double, const int);
>  vector float vec_xxpermdi (vector float, vector float, const int);
> +vector __int128 vec_xxpermdi (vector signed __int128,
> +                              vector signed __int128, const int);

Nit: either s/vector __int128/vector signed __int128/
     or s/signed //g
to keep consistent.

> +vector __int128 vec_xxpermdi (vector unsigned __int128,
> +                              vector unsigned __int128, const int);

This line misses unsigned for the return type.

OK for trunk with nits above tweaked, thanks!

BR,
Kewen

Reply via email to