| Message ID | 4e3c59b0-b6d9-4b29-a65f-c877926a4552@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000, built-in cleanup patch series | expand |
Hi Carl, on 2024/6/14 03:40, Carl Love wrote: > GCC maintainers: > > Per the comments on patch 0004 from version 3, the removal of > The built-in __builtin_vsx_xvcvdpuxds_uns and __builtin_vsx_xvcvspuxws was moved to this patch. The rest of the patch is unchanged from version 3. There were no comments on this patch for version 3. > > Please let me know if this patch is acceptable. Thanks. > > Carl > > > ----------------------------------------------------- > > rs6000, Remove __builtin_vsx_xvcvspsxws, > __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws built-ins. Nit: Maybe make it shorter like: Remove built-ins __builtin_vsx_xvcv{sp{sx,u}ws,dpuxds_uns} > > The built-in __builtin_vsx_xvcvspsxws is a duplicate of the vec_signed Nit: Strictly speaking, not a duplicate of vec_signed but covered by it. > 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. > > The built-in __builtin_vsx_xvcvdpuxds_uns is redundant as it is covered by > vec_unsigned, remove. > > The __builtin_vsx_xvcvspuxws is redundant as it is covered by > vec_unsigned, remove. As mentioned in the previous review, I'd expect patch 4/13 only focuses on extending vec_{un,}signed{e,o} for vector float (aka. __builtin_vsx_xvcvspsxds and __builtin_vsx_xvcvspuxds related), and this patch focuses on some built-in removals which have been covered by the existing vec_{un,}signed{,e,o}, so it can also drop the built-ins: "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." // copied from 4/13. BR, Kewen > > This patch removes the redundant built-in. > > gcc/ChangeLog: > * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxws, > __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws): > Remove built-in definitions. > --- > gcc/config/rs6000/rs6000-builtins.def | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def > index 7c36976a089..8cf0b715898 100644 > --- a/gcc/config/rs6000/rs6000-builtins.def > +++ b/gcc/config/rs6000/rs6000-builtins.def > @@ -1697,9 +1697,6 @@ > const vsll __builtin_vsx_xvcvdpuxds_scale (vd, const int); > XVCVDPUXDS_SCALE vsx_xvcvdpuxds_scale {} > > - const vull __builtin_vsx_xvcvdpuxds_uns (vd); > - XVCVDPUXDS_UNS vsx_fixuns_truncv2dfv2di2 {} > - > const vsi __builtin_vsx_xvcvdpuxws (vd); > XVCVDPUXWS vsx_xvcvdpuxws {} > > @@ -1709,15 +1706,9 @@ > const vsll __builtin_vsx_xvcvspsxds (vf); > XVCVSPSXDS vsx_xvcvspsxds {} > > - const vsi __builtin_vsx_xvcvspsxws (vf); > - XVCVSPSXWS vsx_fix_truncv4sfv4si2 {} > - > const vsll __builtin_vsx_xvcvspuxds (vf); > XVCVSPUXDS vsx_xvcvspuxds {} > > - const vsi __builtin_vsx_xvcvspuxws (vf); > - XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {} > - > const vd __builtin_vsx_xvcvsxddp (vsll); > XVCVSXDDP vsx_floatv2div2df2 {} >
Kewen: On 6/18/24 20:03, Kewen.Lin wrote: > Hi Carl, > > on 2024/6/14 03:40, Carl Love wrote: >> GCC maintainers: >> >> Per the comments on patch 0004 from version 3, the removal of >> The built-in __builtin_vsx_xvcvdpuxds_uns and __builtin_vsx_xvcvspuxws was moved to this patch. The rest of the patch is unchanged from version 3. There were no comments on this patch for version 3. >> >> Please let me know if this patch is acceptable. Thanks. >> >> Carl >> >> >> ----------------------------------------------------- >> >> rs6000, Remove __builtin_vsx_xvcvspsxws, >> __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws built-ins. > > Nit: Maybe make it shorter like: Remove built-ins __builtin_vsx_xvcv{sp{sx,u}ws,dpuxds_uns} > >> >> The built-in __builtin_vsx_xvcvspsxws is a duplicate of the vec_signed > > Nit: Strictly speaking, not a duplicate of vec_signed but covered by it. > >> 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. >> >> The built-in __builtin_vsx_xvcvdpuxds_uns is redundant as it is covered by >> vec_unsigned, remove. >> >> The __builtin_vsx_xvcvspuxws is redundant as it is covered by >> vec_unsigned, remove. > > As mentioned in the previous review, I'd expect patch 4/13 only focuses on > extending vec_{un,}signed{e,o} for vector float (aka. __builtin_vsx_xvcvspsxds > and __builtin_vsx_xvcvspuxds related), and this patch focuses on some built-in > removals which have been covered by the existing vec_{un,}signed{,e,o}, so > it can also drop the built-ins: > > "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." > > // copied from 4/13. Not sure why I didn't move these two with the other two??? Sorry. Moved them from patch 4. Carl
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def index 7c36976a089..8cf0b715898 100644 --- a/gcc/config/rs6000/rs6000-builtins.def +++ b/gcc/config/rs6000/rs6000-builtins.def @@ -1697,9 +1697,6 @@ const vsll __builtin_vsx_xvcvdpuxds_scale (vd, const int); XVCVDPUXDS_SCALE vsx_xvcvdpuxds_scale {} - const vull __builtin_vsx_xvcvdpuxds_uns (vd); - XVCVDPUXDS_UNS vsx_fixuns_truncv2dfv2di2 {} - const vsi __builtin_vsx_xvcvdpuxws (vd); XVCVDPUXWS vsx_xvcvdpuxws {} @@ -1709,15 +1706,9 @@ const vsll __builtin_vsx_xvcvspsxds (vf); XVCVSPSXDS vsx_xvcvspsxds {} - const vsi __builtin_vsx_xvcvspsxws (vf); - XVCVSPSXWS vsx_fix_truncv4sfv4si2 {} - const vsll __builtin_vsx_xvcvspuxds (vf); XVCVSPUXDS vsx_xvcvspuxds {} - const vsi __builtin_vsx_xvcvspuxws (vf); - XVCVSPUXWS vsx_fixuns_truncv4sfv4si2 {} - const vd __builtin_vsx_xvcvsxddp (vsll); XVCVSXDDP vsx_floatv2div2df2 {}
GCC maintainers: Per the comments on patch 0004 from version 3, the removal of The built-in __builtin_vsx_xvcvdpuxds_uns and __builtin_vsx_xvcvspuxws was moved to this patch. The rest of the patch is unchanged from version 3. There were no comments on this patch for version 3. Please let me know if this patch is acceptable. Thanks. Carl ----------------------------------------------------- rs6000, Remove __builtin_vsx_xvcvspsxws, __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws built-ins. 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. The built-in __builtin_vsx_xvcvdpuxds_uns is redundant as it is covered by vec_unsigned, remove. The __builtin_vsx_xvcvspuxws is redundant as it is covered by vec_unsigned, remove. This patch removes the redundant built-in. gcc/ChangeLog: * config/rs6000/rs6000-builtins.def (__builtin_vsx_xvcvspsxws, __builtin_vsx_xvcvdpuxds_uns, __builtin_vsx_xvcvspuxws): Remove built-in definitions. --- gcc/config/rs6000/rs6000-builtins.def | 9 --------- 1 file changed, 9 deletions(-)