On 6/18/24 20:03, Kewen.Lin wrote:
> 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/
Fixed.
>
>> existing vec_{un,}signed{e,o} built-ins to handle the argument
>> vector of floats to return the even/odd signed/unsigned integers.
>>
>
> Likewise.
Fixed.
>
>> 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).
>
Moved to patch 2.
>>
>> 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/
OK, fixed.
>
>> __builtin_vsignede_v4sf, __builtin_vunsignede_v4sf respectively.
>> (XVCVSPSXDS, XVCVSPUXDS): Renamed VEC_VSIGNEDE_V4SF,
>> VEC_VUNSIGNEDE_V4SF respectively.
>
> Likewise.
OK, fixed.
>
>> (__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: "..,.." -> ".., ..", " " -> " "
OK, I fixed the various spacing issues.
>
>> * config/rs6000/vsx.md (vsignede_v4sf, vsignedo_v4sf,
>> vunsignede_v4sf, vunsignedo_v4sf): New define_expands.
>
> Likewise.
dito
>
>> * 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".
>
OK, fixed.
>>
>> 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).
Yes, fixed four instances of the issue.
>
>> + 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.
Fixed.
>
>> + 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).
OK, fixed.
>
>> +(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.
Fixed.
>
>> + 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.
Fixed.
>
>
>> +(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.
Fixed.
>
>> + 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.
Fixed the last two lines so they read:
vector unsigned long long vec_signedo (vector float);
vector unsigned long long vec_signede (vector float);
Carl
