Hi Carl,

on 2024/6/14 03:40, Carl Love wrote:
> 
> GCC maintainers:
> 
> As noted the removal of __builtin_vsx_xvcvdpuxds_uns and 
> __builtin_vsx_xvcvspuxws was moved to patch 2 in the seris.  The patch has 
> been updated per the comments from version 3.
> 
> Please let me know if this patch is acceptable for mainline.  
> 
>                              Carl 
> 
> ------------------------------------------------------------------
> 
> rs6000, extend the current vec_{un,}signed{e,o} built-ins
> 
> The built-ins __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds
> convert a vector of floats to signed/unsigned long long ints.  Extend the

Nit: s/signed/a vector of signed/

> existing vec_{un,}signed{e,o} built-ins to handle the argument
> vector of floats to return the even/odd signed/unsigned integers.
> 

Likewise.

> The define expands vsignede_v4sf, vsignedo_v4sf, vunsignede_v4sf,
> vunsignedo_v4sf are added to support the new vec_{un,}signed{e,o}
> built-ins.
> 
> The built-ins __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds are
> now for internal use only. They are not documented and they do not
> have testcases.
> 


> The built-in __builtin_vsx_xvcvdpsxws is redundant as it is covered by
> vec_signed{e,o}, remove.
> 
> The built-in __builtin_vsx_xvcvdpuxws is redundant as it is covered by
> vec_unsigned{e,o}, remove.

As the comments in 2/13 v4 and the previous review comments, I preferred
these two are moved to 2/13 as well (this patch should focus on extending).

> 
> Add testcases and update documentation.
> 
> gcc/ChangeLog:
>       * config/rs6000/rs6000-builtins.def: __builtin_vsx_xvcvdpsxws,
>       __builtin_vsx_xvcvdpuxws): Removed.
>       (__builtin_vsx_xvcvspsxds, __builtin_vsx_xvcvspuxds): Renamed

Nit: s/Renamed/Rename to/

>       __builtin_vsignede_v4sf, __builtin_vunsignede_v4sf respectively.
>       (XVCVSPSXDS, XVCVSPUXDS): Renamed VEC_VSIGNEDE_V4SF,
>       VEC_VUNSIGNEDE_V4SF respectively.

Likewise.

>       (__builtin_vsignedo_v4sf, __builtin_vunsignedo_v4sf): New
>       built-in definitions.
>       * config/rs6000/rs6000-overload.def (vec_signede, vec_signedo,
>       vec_unsignede,vec_unsignedo):  Add new overloaded specifications.

Formatting nits: "..,.." -> ".., ..", "  " -> " "

>       * config/rs6000/vsx.md (vsignede_v4sf, vsignedo_v4sf,
>       vunsignede_v4sf, vunsignedo_v4sf): New  define_expands.

Likewise.

>       * doc/extend.texi (vec_signedo, vec_signede): Add documentation
>       for new overloaded built-ins.

Missing vec_unsignedo and vec_unsignede, may be also mention for which
types, like "converting vector float to vector {un,}signed long long".

> 
> gcc/testsuite/ChangeLog:
>       * gcc.target/powerpc/builtins-3-runnable.c
>       (test_unsigned_int_result, test_ll_unsigned_int_result): Add
>       new argument.
>       (vec_signede, vec_signedo, vec_unsignede, vec_unsignedo): New
>       tests for the overloaded built-ins.
> ---  gcc/config/rs6000/rs6000-builtins.def         | 20 ++---
>  gcc/config/rs6000/rs6000-overload.def         |  8 ++
>  gcc/config/rs6000/vsx.md                      | 84 +++++++++++++++++++
>  gcc/doc/extend.texi                           | 10 +++
>  .../gcc.target/powerpc/builtins-3-runnable.c  | 49 +++++++++--
>  5 files changed, 154 insertions(+), 17 deletions(-)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def 
> b/gcc/config/rs6000/rs6000-builtins.def
> index 322d27b7a0d..29a9deb3410 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1688,26 +1688,26 @@
>    const vsll __builtin_vsx_xvcvdpsxds_scale (vd, const int);
>      XVCVDPSXDS_SCALE vsx_xvcvdpsxds_scale {}
>  
> -  const vsi __builtin_vsx_xvcvdpsxws (vd);
> -    XVCVDPSXWS vsx_xvcvdpsxws {}
> -
>    const vsll __builtin_vsx_xvcvdpuxds (vd);
>      XVCVDPUXDS vsx_fixuns_truncv2dfv2di2 {}
>  
>    const vsll __builtin_vsx_xvcvdpuxds_scale (vd, const int);
>      XVCVDPUXDS_SCALE vsx_xvcvdpuxds_scale {}
>  
> -  const vsi __builtin_vsx_xvcvdpuxws (vd);
> -    XVCVDPUXWS vsx_xvcvdpuxws {}
> -
>    const vd __builtin_vsx_xvcvspdp (vf);
>      XVCVSPDP vsx_xvcvspdp {}
>  
> -  const vsll __builtin_vsx_xvcvspsxds (vf);
> -    XVCVSPSXDS vsx_xvcvspsxds {}
> +  const vsll __builtin_vsignede_v4sf (vf);
> +    VEC_VSIGNEDE_V4SF vsignede_v4sf {}
> +
> +  const vsll __builtin_vsignedo_v4sf (vf);
> +    VEC_VSIGNEDO_V4SF vsignedo_v4sf {}
> +
> +  const vull __builtin_vunsignede_v4sf (vf);
> +    VEC_VUNSIGNEDE_V4SF vunsignede_v4sf {}
>  
> -  const vsll __builtin_vsx_xvcvspuxds (vf);
> -    XVCVSPUXDS vsx_xvcvspuxds {}
> +  const vull __builtin_vunsignedo_v4sf (vf);
> +    VEC_VUNSIGNEDO_V4SF vunsignedo_v4sf {}
>  
>    const vd __builtin_vsx_xvcvsxddp (vsll);
>      XVCVSXDDP vsx_floatv2div2df2 {}
> diff --git a/gcc/config/rs6000/rs6000-overload.def 
> b/gcc/config/rs6000/rs6000-overload.def
> index 84bd9ae6554..4d857bb1af3 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -3307,10 +3307,14 @@
>  [VEC_SIGNEDE, vec_signede, __builtin_vec_vsignede]
>    vsi __builtin_vec_vsignede (vd);
>      VEC_VSIGNEDE_V2DF
> +  vsll __builtin_vec_vsignede (vf);
> +    VEC_VSIGNEDE_V4SF
>  
>  [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
>    vsi __builtin_vec_vsignedo (vd);
>      VEC_VSIGNEDO_V2DF
> +  vsll __builtin_vec_vsignedo (vf);
> +    VEC_VSIGNEDO_V4SF
>  
>  [VEC_SIGNEXTI, vec_signexti, __builtin_vec_signexti]
>    vsi __builtin_vec_signexti (vsc);
> @@ -4433,10 +4437,14 @@
>  [VEC_UNSIGNEDE, vec_unsignede, __builtin_vec_vunsignede]
>    vui __builtin_vec_vunsignede (vd);
>      VEC_VUNSIGNEDE_V2DF
> +  vull __builtin_vec_vunsignede (vf);
> +    VEC_VUNSIGNEDE_V4SF
>  
>  [VEC_UNSIGNEDO, vec_unsignedo, __builtin_vec_vunsignedo]
>    vui __builtin_vec_vunsignedo (vd);
>      VEC_VUNSIGNEDO_V2DF
> +  vull __builtin_vec_vunsignedo (vf);
> +    VEC_VUNSIGNEDO_V4SF
>  
>  [VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
>    vui __builtin_vec_extract_exp (vf);
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index f135fa079bd..0187bdbf90e 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -2704,6 +2704,90 @@
>    DONE;
>  })
>  
> +;; Convert float vector even elements to signed long long vector
> +(define_expand "vsignede_v4sf"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  if (BYTES_BIG_ENDIAN)
> +    emit_insn (gen_vsx_xvcvspsxds_be (operands[0], operands[1]));
> +  else
> +    {
> +      /* Shift left one word to put even word in correct location */

Nit: s/location/location. /
(two spaces after period).

> +      rtx rtx_tmp = gen_reg_rtx (V4SFmode);
> +      rtx rtx_val = GEN_INT (4);
> +      emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                       rtx_val));
> +      emit_insn (gen_vsx_xvcvspsxds_le (operands[0], rtx_tmp));
> +    }
> +
> +  DONE;
> +})
> +
> +;; Convert float vector odd elements to signed long long vector
> +(define_expand "vsignedo_v4sf"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  if (BYTES_BIG_ENDIAN)
> +    {
> +      /* Shift left one word to put even word in correct location */

Likewise.

> +      rtx rtx_tmp = gen_reg_rtx (V4SFmode);
> +      rtx rtx_val = GEN_INT (4);
> +      emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                       rtx_val));
> +      emit_insn (gen_vsx_xvcvspsxds_be (operands[0], rtx_tmp));
> +    }
> +  else
> +    emit_insn (gen_vsx_xvcvspsxds_le (operands[0], operands[1]));
> +
> +  DONE;
> +})
> +
> +;; Convert even vector elements to unsigned long long vector

Nit: This comment miss "float" (it doesn't align with the one for vsignede_v4sf 
above).

> +(define_expand "vunsignede_v4sf"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  if (BYTES_BIG_ENDIAN)
> +    emit_insn (gen_vsx_xvcvspuxds_be (operands[0], operands[1]));
> +  else
> +    {
> +      /* Shift left one word to put even word in correct location */

Likewise.

> +      rtx rtx_tmp = gen_reg_rtx (V4SFmode);
> +      rtx rtx_val = GEN_INT (4);
> +      emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                       rtx_val));
> +      emit_insn (gen_vsx_xvcvspuxds_le (operands[0], rtx_tmp));
> +    }
> +
> +  DONE;
> +})
> +
> +;; Convert odd vector elements to unsigned long long vector

Likewise.


> +(define_expand "vunsignedo_v4sf"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  if (BYTES_BIG_ENDIAN)
> +    {
> +      /* Shift left one word to put even word in correct location */

Likewise.

> +      rtx rtx_tmp = gen_reg_rtx (V4SFmode);
> +      rtx rtx_val = GEN_INT (4);
> +      emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                       rtx_val));
> +      emit_insn (gen_vsx_xvcvspuxds_be (operands[0], rtx_tmp));
> +    }
> +  else
> +    emit_insn (gen_vsx_xvcvspuxds_le (operands[0], operands[1]));
> +
> +  DONE;
> +})
> +
>  ;; Generate float2 double
>  ;; convert two double to float
>  (define_expand "float2_v2df"
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 799a36586dc..b1620274285 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -22625,6 +22625,16 @@ if the VSX instruction set is available.  The 
> @samp{vec_vsx_ld} and
>  @samp{vec_vsx_st} built-in functions always generate the VSX @samp{LXVD2X},
>  @samp{LXVW4X}, @samp{STXVD2X}, and @samp{STXVW4X} instructions.
>  
> +@smallexample
> +vector signed long long vec_signedo (vector float);
> +vector signed long long vec_signede (vector float);
> +vector unsigned signed long long vec_signedo (vector float);
> +vector unsigned signed long long vec_signede (vector float);

The last two lines should be vec_**un**signed{o,e} and unexpected "signed" 
should be removed.

The others look good, thanks!

BR,
Kewen

Reply via email to