mbox series

[ver2,0/4] rs6000, remove redundant built-ins and add more test cases

Message ID c959f3ff-ed1e-40be-95d0-6a45ab35e8a5@linux.ibm.com
Headers show
Series rs6000, remove redundant built-ins and add more test cases | expand

Message

Carl Love Oct. 1, 2024, 3:12 p.m. UTC 
GCC maintainers:

The following version 2 of a series of patches for PowerPC removes some 
built-ins that are covered by existing overloaded built-ins. 
Additionally, there are patches to add missing testcases and 
documentation.  The original version of the patch series was posted on 
8/7/2024.  It was originally reviewed by Kewen.

The patches have been updated per the review.  Note patches 2 and 3 in 
the series were approved with minor changes.  I will post the entire 
series for review for completeness.

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

Please let me know if the patches are acceptable for mainline. Thanks.

                                         Carl

Comments

Carl Love Oct. 1, 2024, 3:27 p.m. UTC | #1 
GCC maintainers:

Version 2, fixed the changelog, updated the wording in the documentation 
and updated the argument types in the vsx-builtin-3.c test file.

The following patch adds missing test cases for the overloaded vec_perm 
built-in.  It also fixes and issue with printing the 128-bit values in 
the DEBUG section that was noticed when adding the additional test cases.

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

---------------------------------------------------------------------------
 From 4c672e8895107bc1f62e09122e7af157436cb83d Mon Sep 17 00:00:00 2001
From: Carl Love <cel@linux.ibm.com>
Date: Wed, 31 Jul 2024 16:31:34 -0400
Subject: [PATCH 1/4] rs6000, add testcases to the overloaded vec_perm 
built-in

The overloaded vec_perm built-in supports permuting signed and unsigned
vectors of char, bool char, short int, short bool, int, bool, long long
int, long long bool, int128, float and double.  However, not all of the
supported arguments are included in the test cases.  This patch adds
the missing test cases.

Additionally, in the 128-bit debug print statements the expected result and
the result need to be cast to unsigned long long to print correctly.  The
patch makes this additional change to the print statements.

gcc/ChangeLog:
     * doc/extend.texi: Fix spelling mistake in description of the
     vec_sel built-in.  Add documentation of the 128-bit vec_perm
     instance.

gcc/testsuite/ChangeLog:
     * gcc.target/powerpc/vsx-builtin-3.c: Add vec_perm test cases for
     arguments of type vector signed long long int, long long bool,
     bool, bool short, bool char and pixel,    vector unsigned long long
     int, unsigned int, unsigned short int, unsigned char.  Cast
     arguments for debug prints to unsigned long long.
     * gcc.target/powerpc/builtins-4-int128-runnable.c: Add vec_perm
     test cases for signed and unsigned int128 arguments.
---
  gcc/doc/extend.texi                           |  12 +-
  .../powerpc/builtins-4-int128-runnable.c      | 108 +++++++++++++++---
  .../gcc.target/powerpc/vsx-builtin-3.c        |  14 ++-
  3 files changed, 116 insertions(+), 18 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 2d795ba7e59..adc4a54c5fa 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -21642,9 +21642,19 @@ vector bool __int128 vec_sel (vector bool __int128,
                 vector bool __int128, vector unsigned __int128);
  @end smallexample

-The instance is an extension of the exiting overloaded built-in 
@code{vec_sel}
+The instance is an extension of the existing overloaded built-in 
@code{vec_sel}
  that is documented in the PVIPR.

+@smallexample
+vector signed __int128 vec_perm (vector signed __int128,
+               vector signed __int128);
+vector unsigned __int128 vec_perm (vector unsigned __int128,
+               vector unsigned __int128);
+@end smallexample
+
+The instance is an extension of the existing overloaded built-in
+@code{vec_perm} that is documented in the PVIPR.
+
  @node Basic PowerPC Built-in Functions Available on ISA 2.06
  @subsubsection Basic PowerPC Built-in Functions Available on ISA 2.06

diff --git 
a/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c 
b/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
index 62c11132cf3..c61b0ecb854 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-4-int128-runnable.c
@@ -18,6 +18,16 @@ int main() {
    __uint128_t data_u128[100];
    __int128_t data_128[100];

+#ifdef __BIG_ENDIAN__
+  vector unsigned char vuc = {0xC, 0xD, 0xE, 0xF, 0x8, 0x9, 0xA, 0xB,
+                              0x1C, 0x1D, 0x1E, 0x1F, 0x18, 0x19, 0x1A, 
0x1B};
+#else
+  vector unsigned char vuc = {0x4, 0x5, 0x6, 0x7, 0x0, 0x1, 0x2, 0x3,
+                  0x14, 0x15, 0x16, 0x17, 0x10, 0x11, 0x12, 0x13};
+#endif
+
+  vector __int128_t vec_128_arg1, vec_128_arg2;
+  vector __uint128_t vec_u128_arg1, vec_u128_arg2;
    vector __int128_t vec_128_expected1, vec_128_result1;
    vector __uint128_t vec_u128_expected1, vec_u128_result1;
    signed long long zero = (signed long long) 0;
@@ -37,11 +47,13 @@ int main() {
      {
  #ifdef DEBUG
      printf("Error: vec_xl(), vec_128_result1[0] = %lld %llu; ",
-           vec_128_result1[0] >> 64,
-           vec_128_result1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_128_result1[0] >> 64),
+           (unsigned long long)(vec_128_result1[0]
+                    & (__int128_t)0xFFFFFFFFFFFFFFFF));
      printf("vec_128_expected1[0] = %lld %llu\n",
-           vec_128_expected1[0] >> 64,
-           vec_128_expected1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_128_expected1[0] >> 64),
+           (unsigned long long)(vec_128_expected1[0]
+                    & (__int128_t)0xFFFFFFFFFFFFFFFF));
  #else
      abort ();
  #endif
@@ -53,11 +65,13 @@ int main() {
      {
  #ifdef DEBUG
      printf("Error: vec_xl(), vec_u128_result1[0] = %lld; ",
-           vec_u128_result1[0] >> 64,
-           vec_u128_result1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_u128_result1[0] >> 64),
+           (unsigned long long)(vec_u128_result1[0]
+                    & (__int128_t)0xFFFFFFFFFFFFFFFF));
      printf("vec_u128_expected1[0] = %lld\n",
-           vec_u128_expected1[0] >> 64,
-           vec_u128_expected1[0] & (__int128_t)0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_u128_expected1[0] >> 64),
+           (unsigned long long)(vec_u128_expected1[0]
+                    & (__int128_t)0xFFFFFFFFFFFFFFFF));
  #else
      abort ();
  #endif
@@ -76,11 +90,12 @@ int main() {
      {
  #ifdef DEBUG
      printf("Error: vec_xl_be(), vec_128_result1[0] = %llu %llu;",
-           vec_128_result1[0] >> 64,
-           vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_128_result1[0] >> 64),
+           (unsigned long long)(vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF));
      printf(" vec_128_expected1[0] = %llu %llu\n",
-           vec_128_expected1[0] >> 64,
-           vec_128_expected1[0] & 0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_128_expected1[0] >> 64),
+           (unsigned long long)(vec_128_expected1[0]
+                    & 0xFFFFFFFFFFFFFFFF));
  #else
        abort ();
  #endif
@@ -98,11 +113,72 @@ int main() {
      {
  #ifdef DEBUG
      printf("Error: vec_xl_be(), vec_u128_result1[0] = %llu %llu;",
-           vec_u128_result1[0] >> 64,
-           vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_u128_result1[0] >> 64),
+           (unsigned long long)(vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF));
+    printf(" vec_u128_expected1[0] = %llu %llu\n",
+           (unsigned long long)(vec_u128_expected1[0] >> 64),
+           (unsigned long long)(vec_u128_expected1[0]
+                    & 0xFFFFFFFFFFFFFFFF));
+#else
+      abort ();
+#endif
+    }
+
+  /* vec_perm() tests */
+  vec_128_arg1 = (vector __int128_t){ (__uint128_t)0x1122334455667788ULL };
+  vec_128_arg2 = (vector __int128_t){ (__uint128_t)0xAAABBBCCCDDDEEEF };
+
+#ifdef __BIG_ENDIAN__
+  vec_128_expected1[0] = 0x5566778811223344ULL;
+  vec_128_expected1[0] = (vec_128_expected1[0] << 64) |
+    0xcdddeeefaaabbbccULL;
+#else
+  vec_128_expected1[0] = 0xcdddeeefaaabbbccULL;
+  vec_128_expected1[0] = (vec_128_expected1[0] << 64) |
+    0x5566778811223344ULL;
+#endif
+
+  vec_128_result1 = vec_perm (vec_128_arg1, vec_128_arg2, vuc);
+
+  if (vec_128_expected1[0] != vec_128_result1[0])
+    {
+#ifdef DEBUG
+    printf("Error: vec_perm(), vec_128_result1[0] = %llu %llu;",
+           (unsigned long long)(vec_128_result1[0] >> 64),
+           (unsigned long long)(vec_128_result1[0] & 0xFFFFFFFFFFFFFFFF));
+    printf(" vec_128_expected1[0] = %llu %llu\n",
+           (unsigned long long)(vec_128_expected1[0] >> 64),
+           (unsigned long long)(vec_128_expected1[0]
+                    & 0xFFFFFFFFFFFFFFFF));
+#else
+      abort ();
+#endif
+    }
+  vec_u128_arg1 = (vector __uint128_t){ 
(__uint128_t)0x1122334455667788ULL };
+  vec_u128_arg2 = (vector __uint128_t){ (__uint128_t)0xAAABBBCCCDDDEEEF };
+
+#ifdef __BIG_ENDIAN__
+  vec_u128_expected1[0] = 0x5566778811223344ULL;
+  vec_u128_expected1[0] = (vec_u128_expected1[0] << 64) |
+    0xcdddeeefaaabbbccULL;
+#else
+  vec_u128_expected1[0] = 0xcdddeeefaaabbbccULL;
+  vec_u128_expected1[0] = (vec_u128_expected1[0] << 64) |
+    0x5566778811223344ULL;
+#endif
+
+  vec_u128_result1 = vec_perm (vec_u128_arg1, vec_u128_arg2, vuc);
+
+  if (vec_u128_expected1[0] != vec_u128_result1[0])
+    {
+#ifdef DEBUG
+    printf("Error: vec_perm(), vec_u128_result1[0] = %llu %llu;",
+           (unsigned long long)(vec_u128_result1[0] >> 64),
+           (unsigned long long)(vec_u128_result1[0] & 0xFFFFFFFFFFFFFFFF));
      printf(" vec_u128_expected1[0] = %llu %llu\n",
-           vec_u128_expected1[0] >> 64,
-           vec_u128_expected1[0] & 0xFFFFFFFFFFFFFFFF);
+           (unsigned long long)(vec_u128_expected1[0] >> 64),
+           (unsigned long long)(vec_u128_expected1[0]
+                    & 0xFFFFFFFFFFFFFFFF));
  #else
        abort ();
  #endif
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c 
b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
index 67c93be1469..9e5a086930b 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-builtin-3.c
@@ -39,6 +39,8 @@

  #include <altivec.h>

+extern __vector long long bool bll[][4];
+extern __vector unsigned long long ull[][4];
  extern __vector int si[][4];
  extern __vector short ss[][4];
  extern __vector signed char sc[][4];
@@ -55,7 +57,6 @@ extern __vector double d[][4];
  extern __vector long sl[][4];
  extern __vector long long sll[][4];
  extern __vector unsigned long ul[][4];
-extern __vector unsigned long long ull[][4];
  extern __vector __bool long bl[][4];
  #endif

@@ -88,12 +89,23 @@ int do_perm(void)
  {
    int i = 0;

+  sll[i][0] = vec_perm (sll[i][1], sll[i][2], uc[i][3]); i++;
+  bll[i][0] = vec_perm (bll[i][1], bll[i][2], uc[i][3]); i++;
    si[i][0] = vec_perm (si[i][1], si[i][2], uc[i][3]); i++;
+  bi[i][0] = vec_perm (bi[i][1], bi[i][2], uc[i][3]); i++;
    ss[i][0] = vec_perm (ss[i][1], ss[i][2], uc[i][3]); i++;
+  bs[i][0] = vec_perm (bs[i][1], bs[i][2], uc[i][3]); i++;
    sc[i][0] = vec_perm (sc[i][1], sc[i][2], uc[i][3]); i++;
+  bc[i][0] = vec_perm (bc[i][1], bc[i][2], uc[i][3]); i++;
+  p[i][0] = vec_perm (p[i][1], p[i][2], uc[i][3]); i++;
    f[i][0] = vec_perm (f[i][1], f[i][2], uc[i][3]); i++;
    d[i][0] = vec_perm (d[i][1], d[i][2], uc[i][3]); i++;

+  ull[i][0] = vec_perm (ull[i][1], ull[i][2], uc[i][3]); i++;
+  ui[i][0] = vec_perm (ui[i][1], ui[i][2], uc[i][3]); i++;
+  us[i][0] = vec_perm (us[i][1], us[i][2], uc[i][3]); i++;
+  uc[i][0] = vec_perm (uc[i][1], uc[i][2], uc[i][3]); i++;
+
    si[i][0] = vec_perm (si[i][1], si[i][2], uc[i][3]); i++;
    ss[i][0] = vec_perm (ss[i][1], ss[i][2], uc[i][3]); i++;
    sc[i][0] = vec_perm (sc[i][1], sc[i][2], uc[i][3]); i++;
Carl Love Oct. 1, 2024, 3:27 p.m. UTC | #2 
GCC maintainers:

version 2, added the reference to the patch where the removal of the 
built-ins was missed.  Note, patch was approved by Kewen with this change.

The following patch removes two redundant built-ins 
__builtin_vsx_vperm_8hi and __builtin_vsx_vperm_8hi_uns.  The built-ins 
are covered by the overloaded vec_perm built-in.

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, remove built-ins __builtin_vsx_vperm_8hi and 
__builtin_vsx_vperm_8hi_uns

The two built-ins __builtin_vsx_vperm_8hi and __builtin_vsx_vperm_8hi_uns
are redundant. The are covered by the overloaded vec_perm built-in. The
built-ins are not documented and do not have test cases.

The removal of these built-ins was missed in commit gcc r15-1923 on
7/9/2024.

This patch removes the redundant built-ins.

gcc/ChangeLog:
     * config/rs6000/rs6000-builtins.def (__builtin_vsx_vperm_8hi,
     __builtin_vsx_vperm_8hi_uns): Remove built-in definitions.
---
  gcc/config/rs6000/rs6000-builtins.def | 6 ------
  1 file changed, 6 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-builtins.def 
b/gcc/config/rs6000/rs6000-builtins.def
index 0e9dc05dbcf..adb4fe761f3 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1472,12 +1472,6 @@
    const vf __builtin_vsx_uns_floato_v2di (vsll);
      UNS_FLOATO_V2DI unsfloatov2di {}

-  const vss __builtin_vsx_vperm_8hi (vss, vss, vuc);
-    VPERM_8HI_X altivec_vperm_v8hi {}
-
-  const vus __builtin_vsx_vperm_8hi_uns (vus, vus, vuc);
-    VPERM_8HI_UNS_X altivec_vperm_v8hi_uns {}
-
    const vsll __builtin_vsx_vsigned_v2df (vd);
      VEC_VSIGNED_V2DF vsx_xvcvdpsxds {}