Kewen:
On 5/14/24 00:53, Kewen.Lin wrote:
> 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.
Yes, the new tests are missing. Not sure what happened to them. Fixed.
>
>> ---
>> 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, ...
Fixed.
>
>>
>> + const vsll __builtin_vsx_xvcvspuxds_low (vf);
>> + XVCVSPUXDSO vsx_xvcvspuxds_low {}
>
> ... so this copied one should be vull too.
Fixed.
>
> 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
Yes, sounds reasonable. Changed XVCVSPUXDS -> VEC_VUNSIGNEDE_V4SF
XVCVSPUXDSO -> VEC_VUNSIGNEDO_V4SF
XVCVSPSXDS -> VEC_VSIGNEDE_V4SF
XVCVSPSXDSO -> VEC_VSIGNEDO_V4SF
NEED TO ADDRESS RESPONSE TO QUESTION I ASKED.
>
>> 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.
Fixed.
>
>> [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
Yes, I tested builtins-3-runnable.c on a BE machine and it fails. I moved the
shift to the LE stanza only. The patch now passes on LE and BE. I think the
fact that the tests cases got lost is why this didnt' get caught the first time.
>
>> + 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.
Moved, description now just says they are new extensions of the exiting
built-ins documented in the
PVIPR.
Carl
>
> BR,
> Kewen
>
>> +
>> Only functions excluded from the PVIPR are listed here.
>>
>> @smallexample
