Hi Carl,
on 2024/5/25 04:18, Carl Love wrote:
> Kewen:
>
> On 5/14/24 01:43, Kewen.Lin wrote:
>> Hi,
>>
>> on 2024/4/20 05:17, Carl Love wrote:
>>> rs6000, Remove __builtin_vsx_xvcvspsxws built-in
>>>
>>> The built-in __builtin_vsx_xvcvspsxws is a duplicate of the vec_signed
>>> built-in that is documented in the PVIPR. The __builtin_vsx_xvcvspsxws
>>> built-in is not documented and there are no test cases for it.
>>>
>>> This patch removes the redundant built-in.
>>
>> By revisiting the comments on the previous version:
>> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646723.html
>
> The comments from the previous version:
> -----------------------------------------------------------------
> I think we should recommend users to adopt the recommended built-ins in
> PVIPR, by checking the corresponding mnemonic in PVIPR, I got:
>
> __builtin_vsx_xvcvspsxws -> vec_signed
> __builtin_vsx_xvcvspsxds -> N/A
> __builtin_vsx_xvcvspuxds -> N/A
> __builtin_vsx_xvcvdpsxws -> vec_signed{e,o}
> __builtin_vsx_xvcvdpuxws -> vec_unsigned{e,o}
> __builtin_vsx_xvcvdpuxds_uns -> vec_unsigned
> __builtin_vsx_xvcvspdp -> vec_double{e,o}
> __builtin_vsx_xvcvdpsp -> vec_float{e,o}
> __builtin_vsx_xvcvspuxws -> vec_unsigned
> __builtin_vsx_xvcvsxwdp -> vec_double{e,o}
> __builtin_vsx_xvcvuxddp_uns> vec_double
>
> For __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds which don't have
> the according PVIPR built-ins, we can extend the current
> vec_{un,}signed{e,o}
> to cover them and document them following the section mentioning PVIPR.
>
> are handled by multiple patches in the new series. The main comment on the
> previous patch series was to remove most of the built-ins as they were
> redundant. So, basically most of the patches in the previous series were
> thrown out and a new series to remove the built-ins in the current series.
> ----------------------------------------------------------------------------
>
> That all said, I distinctly remember addressing each of the above built-ins.
> The work on the series got
> interrupted a couple of times and it looks like some of the patches to
> address the above got lost. My bad.
> The following is a list of which patch takes care of removing the duplicate
> built-ins.
No problem, thanks for working on this!!
>
> __builtin_vsx_xvcvspsxws patch 2 removes this built-in
> __builtin_vsx_xvcvspsxds -> N/A patch 4 extends vec_{un,}signede
> to cover this built-in,
> Built-in used in
> rs6000-overload.def. Built-in now for
> internal use only.
> __builtin_vsx_xvcvspuxds -> N/A patch 4 extends vec_{un,}signedo
> to cover this built-in.
> Built-in used in
> rs6000-overload.def. Built-in now for
> internal use only
>
>
> __builtin_vsx_xvcvdpsxws -> vec_signed{e,o} removed in patch 4
> __builtin_vsx_xvcvdpuxws -> vec_unsigned{e,o} removed in patch 4
>
> __builtin_vsx_xvcvdpuxds_uns -> vec_unsigned remove in patch 4
> __builtin_vsx_xvcvspuxws -> vec_unsigned remove in patch 4
Just to avoid some misunderstanding, I guess you meant the new patch 4?
As the current patch 4 doesn't remove these.
>
> The following will changes will be put into a new patch when the series is
> reposted. It appears they
> got lost in the current series. My bad.
>
> __builtin_vsx_xvcvspdp -> vec_double{e,o} remove in new patch number 5
> __builtin_vsx_xvcvdpsp -> vec_float{e,o} remove in new patch number 5
>
> __builtin_vsx_xvcvsxwdp -> vec_double{e,o} remove in new patch number 5
> __builtin_vsx_xvcvuxddp_uns> vec_double remove in new patch number 5
>
>>
>> I wonder if it's intentional to keep the others, at least bifs
>> __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws and
>> __builtin_vsx_xvcvuxddp_uns looks removable, users can just uses the
>> equivalent ones in PVIPR. And for the others, users can still use
>> the PVIPR ones by considering endianness (controlling with endianness
>> macros).
>>
>
> Hopefully that makes it clearer where the various changes are.
>
> The next series will add a new patch 5 in the series. The remaining patches
> in this series, patches 5, 6, ... will get moved to patch 6, 7, ... in the
> next posting of the built-in cleanup patch series.
>
OK, thanks!
BR,
Kewen
