GCC maintainers:

Here is my respnses to the review comments by Kewen.  Unfortunately, Kewen is no longer working on GCC power.

I will submit an updated version of the patch with Kewen's suggested changes.

                                     Carl


On 8/20/24 12:54 AM, Kewen.Lin wrote:
Hi Carl,

on 2024/8/8 01:15, Carl Love wrote:

  GCC maintainers:

The following patch fixes errors in the definition of the 
__builtin_vsx_uns_floate_v2di, __builtin_vsx_uns_floato_v2di and 
__builtin_vsx_uns_float2_v2di built-ins.  The arguments should be unsigned but 
are listed as signed.

Additionally, there are a number of test cases that are missing for the various 
instances of the built-ins.  Additionally, the documentation for the various 
built-ins is missing.

This patch adds the missing test cases and documentation.

The patch has been tested on Power 10 LE and BE with no regressions.

Please let me know if it is acceptable for mainline.  Thanks.

                                             Carl
-------------------------------------------------------------------------------------
rs6000, Add tests and documentation for vector conversions between integer and 
float

The arguments for the __builtin_vsx_uns_floate_v2di,
__builtin_vsx_uns_floato_v2di and __builtin_vsx_uns_float2_v2di built-ins
should be unsigned.

Add tests for the following existing integer and long long int to float
built-ins:
   __builtin_altivecfloat_sisf (vsi);
   __builtin_altivec_uns_float_sisf (vui);
   __builtin_vsxfloate_v2di (vsll);
   __builtin_vsx_uns_floate_v2di (vull);
   __builtin_vsx_floato_v2di (vsll);
   __builtin_vsx_uns_floato_v2di (vull);
   __builtin_vsx_float2_v2di (vsll, vsll);
   __builtin_vsx_uns_float2_v2di (vull, vull);

Add tests for the vector float to vector int built-ins:
   __builtin_altivec_fix_sfsi
   __builtin_altivec_fixuns_sfsi

The various built-ins are not documented.  The patch adds the missing
documentation for the variouls built-ins.

This patch fixes the incorrect __builtin_vsx_uns_float[o|e|2]_v2di
argument types and adds test cases for each of the built-ins listed above.

gcc/ChangeLog:
     * config/rs6000/rs6000-builtins.def (__builtin_vsx_uns_floate_v2di,
     __builtin_vsx_uns_floato_v2di,__builtin_vsx_uns_float2_v2di): Change
     argument from signed to unsigned.
     * doc/extend.texi: Add documentation for each of the built-ins.

gcc/testsuite/ChangeLog:
     * gcc.target/powerpc/vsx-int-to-float-runnable.c: New file.
---
  gcc/config/rs6000/rs6000-builtins.def         |   6 +-
  gcc/doc/extend.texi                           |  37 +++
  .../powerpc/vsx-int-to-float-runnable.c       | 260 ++++++++++++++++++
  3 files changed, 300 insertions(+), 3 deletions(-)
  create mode 100644 
gcc/testsuite/gcc.target/powerpc/vsx-int-to-float-runnable.c

diff --git a/gcc/config/rs6000/rs6000-builtins.def 
b/gcc/config/rs6000/rs6000-builtins.def
index f2bebd299b2..1227daa1555 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1463,10 +1463,10 @@
    const vd __builtin_vsx_uns_doubleo_v4si (vsi);
      UNS_DOUBLEO_V4SI unsdoubleov4si2 {}
I noticed there are extra four that should be updated together:

    const vd __builtin_vsx_uns_doublee_v4si (vsi);
      UNS_DOUBLEE_V4SI unsdoubleev4si2 {}

    const vd __builtin_vsx_uns_doubleh_v4si (vsi);
      UNS_DOUBLEH_V4SI unsdoublehv4si2 {}

    const vd __builtin_vsx_uns_doublel_v4si (vsi);
      UNS_DOUBLEL_V4SI unsdoublelv4si2 {}

    const vd __builtin_vsx_uns_doubleo_v4si (vsi);
      UNS_DOUBLEO_V4SI unsdoubleov4si2 {}

Yes, those definitions are also incorrect.  Fixed.

-  const vf __builtin_vsx_uns_floate_v2di (vsll);
+  const vf __builtin_vsx_uns_floate_v2di (vull);
      UNS_FLOATE_V2DI unsfloatev2di {}

-  const vf __builtin_vsx_uns_floato_v2di (vsll);
+  const vf __builtin_vsx_uns_floato_v2di (vull);
      UNS_FLOATO_V2DI unsfloatov2di {}

    const vsll __builtin_vsx_vsigned_v2df (vd);
@@ -2272,7 +2272,7 @@
    const vss __builtin_vsx_revb_v8hi (vss);
      REVB_V8HI revb_v8hi {}

-  const vf __builtin_vsx_uns_float2_v2di (vsll, vsll);
+  const vf __builtin_vsx_uns_float2_v2di (vull, vull);
      UNS_FLOAT2_V2DI uns_float2_v2di {}

    const vsi __builtin_vsx_vsigned2_v2df (vd, vd);
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index bf6f4094040..7ec4f19a6bf 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -22919,6 +22919,43 @@ but the index value must be 0.

  Only functions excluded from the PVIPR are listed here.

+The following built-ins convert signed and unsigned vectors of ints and
+long long ints to a vector of 32-bit floating point values.
+
+@smallexample
+vector float __builtin_altivec_float_sisf (vector int);
+vector float __builtin_altivec_uns_float_sisf (vector unsigned int);
These functions are to convert vector {un,}signed int to vector float,
PVIPR has defined "vec_float" for this kind of conversion.  For now,
this function only considers VSX:

[VEC_FLOAT, vec_float, __builtin_vec_float]
   vf __builtin_vec_float (vsi);
     XVCVSXWSP
   vf __builtin_vec_float (vui);
     XVCVUXWSP

I think we should fix it to have it also supported without VSX but with
ALTIVEC.  We can change the associated instance XVCVSXWSP with
FLOAT_V4SI_V4SF, and update its associated expander floatv4siv4sf2 to
consider VSX support, such as:

if (<MODE>mode == V4SFmode)
   {
     if (VECTOR_UNIT_VSX_P (<MODE>mode))
       emit_insn (gen_vsx_floatv4siv4sf2 (operands[0], operands[1]));
     else
       emit_insn (gen_altivec_vcfsx (operands[0], operands[1], const0_rtx));
     DONE;
   }

(untested), then vec_float can route to altivec_vcfsx with only ALTIVEC
but not VSX, and to XVCVSXWSP with VSX supported.

Similar for XVCVUXWSP with UNSFLOAT_V4SI_V4SF and expander
floatunsv4siv4sf2, and I think we can drop XVCVSXWSP and XVCVUXWSP
definitions as well as they gets only used for overloading.

OK, I have implemented the code needed to generate the VSX instruction if VSX is available and issue the Altivec instruction otherwise.
+vector float __builtin_vsx_floate_v2di (vector signed long long int);
+vector float __builtin_vsx_uns_floate_v2di (vector unsigned long long int);
These two are instances of overloaded function vec_floate, see the definition
of vec_floate below, so I think we shouldn't document them and users should
use "vec_floate" which is well defined in PVIPR.

[VEC_FLOATE, vec_floate, __builtin_vec_floate]
   vf __builtin_vec_floate (vsll);
     FLOATE_V2DI
   vf __builtin_vec_floate (vull);
     UNS_FLOATE_V2DI
   vf __builtin_vec_floate (vd);
     FLOATE_V2DF

Yes, updated patch to remove them.
vector float __builtin_vsx_floato_v2di (vector signed long long int);
+vector float __builtin_vsx_uns_floato_v2di (vector unsigned long long int);
Similarly, these are instances of vec_floato, users should use vec_floato
instead.

Ditto
+vector float __builtin_vsx_float2_v2di (vector signed long long int,
+                                        vector signed long long int);
+vector float __builtin_vsx_uns_float2_v2di (vector unsigned long long int,
+                                            vector signed long long int);
Similarly, these are instances of vec_float2, users should use vec_float2
instead.

Ditto.

                                  Carl

Reply via email to