Re: [PATCH 4/13] rs6000, extend the current vec_{un,}signed{e,o} built-ins

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

Hi,

on 2024/4/20 05:17, Carl Love wrote:
> 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
> existing vec_{un,}signed{e,o} built-ins to handle the argument
> vector of floats to return the even/odd signed/unsigned integers.
> 
> Add testcases and update documentation.
> 
> gcc/ChangeLog:
>         * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxds_low,
>         __builtin_vsx_xvcvspuxds_low): New built-in definitions.
>         * config/rs6000/rs6000-overload.def (vec_signede, vec_signedo):
>         Add new overloaded specifications.
>         * config/rs6000/vsx.md (vsx_xvcvsp<su>xds_low): New define_expand.
>         * doc/extend.texi (vec_signedo, vec_signede): Add documentation.
> 
> gcc/testsuite/ChangeLog:
>         * gcc.target/powerpc/builtins-3-runnable: New tests for the added
>         overloaded built-ins.

This part is missing, there are no test case changes in this patch.

> ---
>  gcc/config/rs6000/rs6000-builtins.def |  6 ++++++
>  gcc/config/rs6000/rs6000-overload.def |  8 ++++++++
>  gcc/config/rs6000/vsx.md              | 23 +++++++++++++++++++++++
>  gcc/doc/extend.texi                   | 13 +++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index bf9a0ae22fc..5b7237a2327 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -1709,9 +1709,15 @@
>    const vsll __builtin_vsx_xvcvspsxds (vf);
>      XVCVSPSXDS vsx_xvcvspsxds {}
>  
> +  const vsll __builtin_vsx_xvcvspsxds_low (vf);
> +    XVCVSPSXDSO vsx_xvcvspsxds_low {}
> +
>    const vsll __builtin_vsx_xvcvspuxds (vf);
>      XVCVSPUXDS vsx_xvcvspuxds {}

This existing should return with type vull, ...

>  
> +  const vsll __builtin_vsx_xvcvspuxds_low (vf);
> +    XVCVSPUXDSO vsx_xvcvspuxds_low {}

... so this copied one should be vull too.

As the existing instances for vec_signed and vec_unsigned are with
names like VEC_V{UN,}SIGNED{O,E}_V2DF, I prefer these are updated
with similar style, maybe something like:

VEC_V{UN,}SIGNED{E,O}_V4SF v{un,}signed{e,o}_v4sf

>    const vsi __builtin_vsx_xvcvspuxws (vf);
>      XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {}
>  > diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index 84bd9ae6554..68501c05289 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);
> +    XVCVSPSXDS
>  
>  [VEC_SIGNEDO, vec_signedo, __builtin_vec_vsignedo]
>    vsi __builtin_vec_vsignedo (vd);
>      VEC_VSIGNEDO_V2DF
> +  vsll __builtin_vec_vsignedo (vf);
> +    XVCVSPSXDSO
>  
>  [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);
> +    XVCVSPUXDS
>  
>  [VEC_UNSIGNEDO, vec_unsignedo, __builtin_vec_vunsignedo]
>    vui __builtin_vec_vunsignedo (vd);
>      VEC_VUNSIGNEDO_V2DF
> +  vull __builtin_vec_vunsignedo (vf);
> +    XVCVSPUXDSO
>  
As above, the name can be tweaked.

>  [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..3d39ae7995f 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -2704,6 +2704,29 @@
>    DONE;
>  })
>  
> +;; Convert low vector elements of 32-bit floating point numbers to vector of
> +;; 64-bit signed/unsigned integers.
> +(define_expand "vsx_xvcvsp<su>xds_low"
> +  [(match_operand:V2DI 0 "vsx_register_operand")
> +   (match_operand:V4SF 1 "vsx_register_operand")
> +   (any_fix (pc))]
> +  "VECTOR_UNIT_VSX_P (V2DFmode)"
> +{
> +  /* Shift left one word to put even word in correct location */
> +  rtx rtx_tmp;
> +  rtx rtx_val = GEN_INT (4);
> +  rtx_tmp = gen_reg_rtx (V4SFmode);
> +  emit_insn (gen_altivec_vsldoi_v4sf (rtx_tmp, operands[1], operands[1],
> +                                      rtx_val));
> +

I think this shift is only needed for LE, see the existing handlings on 
float/signed int to double conversions, like:

(define_expand "doublee<mode>2"
(define_expand "doubleo<mode>2"

> +  if (BYTES_BIG_ENDIAN)
> +    emit_insn (gen_vsx_xvcvsp<su>xds_be (operands[0], rtx_tmp));
> +  else
> +    emit_insn (gen_vsx_xvcvsp<su>xds_le (operands[0], rtx_tmp));
> +
> +  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 7b54a241a7b..64a43b55e2d 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -22552,6 +22552,19 @@ can use @var{vector long} instead of @var{vector long long},
>  @var{vector bool long} instead of @var{vector bool long long}, and
>  @var{vector unsigned long} instead of @var{vector unsigned long long}.
>  
> +@smallexample
> +vector signed signed long long vec_signedo (vector float);
> +vector signed 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);
> +@end smallexample
> +
> +The overloaded built-ins @code{vec_signedo} and @code{vec_signede} convert the
> +even/odd input vector elements to signed/unsigned long long integer values in
> +addition to the supported arguments and return types documented in the PVIPR.
> +Negative input values are returned as zero for the unsigned long long return
> +values.

These functions are placed in stanza [vsx] instead of [power8-vector], so it should
be in the section "PowerPC AltiVec Built-in Functions Available on ISA 2.06".

As mentioned in the replies to other patches, since it's an extension from the
existing PVIPR bifs and there is nothing special from them, I prefer to just mention
them but omit the description on them.

BR,
Kewen

> +
>  Only functions excluded from the PVIPR are listed here.
>  
>  @smallexample