This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, rs6000 V4] Fixes for commit 254464


GCC Maintainers:

The recent commit 254464 to add Power 8 support for the vec_revb()
builtin had a couple of issues that I missed on my testing of the
original patch.  First, on Power 8 BE the le_swap1 permute vector value
was wrong in function swap_endian_selector_for_mode(). This issue caused
the test case builtins-revb-runnable.c to fail on Power 8 BE. Originally
the function was used for all of the various modes including V16QI.
However, a revision of the patch mapped the V16QI case to a move as the
permute is actually a NOP.  Hence this case is not needed in the
function.  With these changes, the function values for
{l,b}e_swap{1,2,4,8} are identical.  Thus the if statement for LE and BE
can be collapsed.  

The additional issues were found with an existing testcase p9-xxbr-1.c
on AIX and Power 9 LE.  The test case was not being properly limited to
run on Power 9 with the new Power 8 support.  Additionally, the change
to map the V16QI to a move results means the test no longer generates
the xxbrq instructions.  The dg directive lp64 is needed on AIX.

I have tested the attached patch to address the above issues.  I have
tested the patch on

 powerpc64-unknown-linux-gnu (Power 8 BE), 
 powerpc64le-unknown-linux-gnu (Power 8 LE),   
 powerpc64le-unknown-linux-gnu (Power 9 LE) 

To verify the issues with two testcases are fixed and no new issues were
created.  My apologies for missing these issues on the original patch.

Please let me know if the following patch is acceptable.  Thanks.

                       Carl Love
---------------------------------------------

gcc/ChangeLog:

2017-11-09  Carl Love  <cel@us.ibm.com>

	* config/rs6000/rs6000.c (swap_endian_selector_for_mode): Remove
	le_ and be_ prefixes to swap* variables.  Remove
	if (VECTOR_ELT_ORDER_BIG) statement. Remove E_V16QImode case statements.

gcc/testsuite/ChangeLog:

2017-11-09  Carl Love  <cel@us.ibm.com>

	* builtins-revb-runnable.c (dg-do run): Add lp64 directive. Fix
	indentation of printf and abort statements.
	* p9-xxbr-1.c (dg-do compile): Add lp64 && p9vector_h directives.
---
 gcc/config/rs6000/rs6000.c                         | 81 +++++++---------------
 .../gcc.target/powerpc/builtins-revb-runnable.c    | 68 +++++++++---------
 gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c       | 11 ++-
 3 files changed, 62 insertions(+), 98 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index aacf3f1..fa2c551 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -14296,66 +14296,33 @@ swap_selector_for_mode (machine_mode mode)
 rtx
 swap_endian_selector_for_mode (machine_mode mode)
 {
-  unsigned int le_swap1[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-  unsigned int le_swap2[16] = {7,6,5,4,3,2,1,0,15,14,13,12,11,10,9,8};
-  unsigned int le_swap4[16] = {3,2,1,0,7,6,5,4,11,10,9,8,15,14,13,12};
-  unsigned int le_swap8[16] = {1,0,3,2,5,4,7,6,9,8,11,10,13,12,15,14};
-  unsigned int le_swap16[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-
-  unsigned int be_swap1[16] = {15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0};
-  unsigned int be_swap2[16] = {7,6,5,4,3,2,1,0,15,14,13,12,11,10,9,8};
-  unsigned int be_swap4[16] = {3,2,1,0,7,6,5,4,11,10,9,8,15,14,13,12};
-  unsigned int be_swap8[16] = {1,0,3,2,5,4,7,6,9,8,11,10,13,12,15,14};
-  unsigned int be_swap16[16] = {15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0};
+  unsigned int swap1[16] = {15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0};
+  unsigned int swap2[16] = {7,6,5,4,3,2,1,0,15,14,13,12,11,10,9,8};
+  unsigned int swap4[16] = {3,2,1,0,7,6,5,4,11,10,9,8,15,14,13,12};
+  unsigned int swap8[16] = {1,0,3,2,5,4,7,6,9,8,11,10,13,12,15,14};
+
   unsigned int *swaparray, i;
   rtx perm[16];
 
-  if (VECTOR_ELT_ORDER_BIG)
-    switch (mode)
-      {
-      case E_V1TImode:
-	swaparray = le_swap1;
-	break;
-      case E_V2DFmode:
-      case E_V2DImode:
-	swaparray = le_swap2;
-	break;
-      case E_V4SFmode:
-      case E_V4SImode:
-	swaparray = le_swap4;
-	break;
-      case E_V8HImode:
-	swaparray = le_swap8;
-	break;
-      case E_V16QImode:
-	swaparray = le_swap16;
-	break;
-      default:
-	gcc_unreachable ();
-      }
-  else
-    switch (mode)
-      {
-      case E_V1TImode:
-	swaparray = be_swap1;
-	break;
-      case E_V2DFmode:
-      case E_V2DImode:
-	swaparray = be_swap2;
-	break;
-      case E_V4SFmode:
-      case E_V4SImode:
-	swaparray = be_swap4;
-	break;
-      case E_V8HImode:
-	swaparray = be_swap8;
-	break;
-      case E_V16QImode:
-	swaparray = be_swap16;
-	break;
-      default:
-	gcc_unreachable ();
-      }
+  switch (mode)
+    {
+    case E_V1TImode:
+      swaparray = swap1;
+      break;
+    case E_V2DFmode:
+    case E_V2DImode:
+      swaparray = swap2;
+      break;
+    case E_V4SFmode:
+    case E_V4SImode:
+      swaparray = swap4;
+      break;
+    case E_V8HImode:
+      swaparray = swap8;
+      break;
+    default:
+      gcc_unreachable ();
+    }
 
   for (i = 0; i < 16; ++i)
     perm[i] = GEN_INT (swaparray[i]);
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-revb-runnable.c b/gcc/testsuite/gcc.target/powerpc/builtins-revb-runnable.c
index 25bd4a2..b6ffa23 100644
--- a/gcc/testsuite/gcc.target/powerpc/builtins-revb-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-revb-runnable.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { powerpc*-*-* && p8vector_hw } } } */
+/* { dg-do run { target { powerpc*-*-* && { lp64 && p8vector_hw } } } } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
 /* { dg-options "-mcpu=power8  -O3" } */
 
@@ -52,10 +52,10 @@ main()
   for (i = 0; i < 16; i++) {
     if (result_bc[i] != expected_bc[i])
 #ifdef DEBUG
-       printf("arg_bc[%d] = 0x%x, result_bc[%d] = 0x%x, expected_bc[%d] = 0x%x\n",
-	      i, arg_bc[i], i, result_bc[i], i, expected_bc[i]);
+      printf("arg_bc[%d] = 0x%x, result_bc[%d] = 0x%x, expected_bc[%d] = 0x%x\n",
+	     i, arg_bc[i], i, result_bc[i], i, expected_bc[i]);
 #else
-    abort();
+      abort();
 #endif
   }
 
@@ -70,10 +70,10 @@ main()
   for (i = 0; i < 16; i++) {
     if (result_uc[i] != expected_uc[i])
 #ifdef DEBUG
-       printf("arg_uc[%d] = 0x%x, result_uc[%d] = 0x%x, expected_uc[%d] = 0x%x\n",
-	      i, arg_uc[i], i, result_uc[i], i, expected_uc[i]);
+      printf("arg_uc[%d] = 0x%x, result_uc[%d] = 0x%x, expected_uc[%d] = 0x%x\n",
+	     i, arg_uc[i], i, result_uc[i], i, expected_uc[i]);
 #else
-    abort();
+      abort();
 #endif
   }
 
@@ -88,10 +88,10 @@ main()
   for (i = 0; i < 16; i++) {
     if (result_sc[i] != expected_sc[i])
 #ifdef DEBUG
-      printf("arg_sc[%d] = 0x%x, result_sc[%d] = 0x%x, expected_sc[%d] = 0x%x\n",
+	printf("arg_sc[%d] = 0x%x, result_sc[%d] = 0x%x, expected_sc[%d] = 0x%x\n",
 	     i, arg_sc[i], i, result_sc[i], i, expected_sc[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -106,10 +106,10 @@ main()
   for (i = 0; i < 8; i++) {
     if (result_bsi[i] != expected_bsi[i])
 #ifdef DEBUG
-       printf("arg_bsi[%d] = 0x%x, result_bsi[%d] = 0x%x, expected_bsi[%d] = 0x%x\n",
+	printf("arg_bsi[%d] = 0x%x, result_bsi[%d] = 0x%x, expected_bsi[%d] = 0x%x\n",
 	      i, arg_bsi[i], i, result_bsi[i], i, expected_bsi[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -123,10 +123,10 @@ main()
   for (i = 0; i < 8; i++) {
     if (result_usi[i] != expected_usi[i])
 #ifdef DEBUG
-      printf("arg_usi[%d] = 0x%x, result_usi[%d] = 0x%x, expected_usi[%d] = 0x%x\n",
+	printf("arg_usi[%d] = 0x%x, result_usi[%d] = 0x%x, expected_usi[%d] = 0x%x\n",
 	     i, arg_usi[i], i, result_usi[i], i, expected_usi[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -140,10 +140,10 @@ main()
   for (i = 0; i < 8; i++) {
     if (result_si[i] != expected_si[i])
 #ifdef DEBUG
-       printf("arg_si[%d] = 0x%x, result_si[%d] = 0x%x, expected_si[%d] = 0x%x\n",
-	      i, arg_si[i], i, result_si[i], i, expected_si[i]);
+	printf("arg_si[%d] = 0x%x, result_si[%d] = 0x%x, expected_si[%d] = 0x%x\n",
+	     i, arg_si[i], i, result_si[i], i, expected_si[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -157,10 +157,10 @@ main()
   for (i = 0; i < 4; i++) {
     if (result_bi[i] != expected_bi[i])
 #ifdef DEBUG
-      printf("arg_bi[%d] = 0x%x, result_bi[%d] = 0x%x, expected_bi[%d] = 0x%x\n",
+	printf("arg_bi[%d] = 0x%x, result_bi[%d] = 0x%x, expected_bi[%d] = 0x%x\n",
 	     i, arg_bi[i], i, result_bi[i], i, expected_bi[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -174,10 +174,10 @@ main()
   for (i = 0; i < 4; i++) {
     if (result_ui[i] != expected_ui[i])
 #ifdef DEBUG
-      printf("arg_ui[%d] = 0x%x, result_ui[%d] = 0x%x, expected_ui[%d] = 0x%x\n",
+	printf("arg_ui[%d] = 0x%x, result_ui[%d] = 0x%x, expected_ui[%d] = 0x%x\n",
 	     i, arg_ui[i], i, result_ui[i], i, expected_ui[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -189,10 +189,10 @@ main()
   for (i = 0; i < 4; i++) {
     if (result_int[i] != expected_int[i])
 #ifdef DEBUG
-      printf("arg_int[%d] = 0x%x, result_int[%d] = 0x%x, expected_int[%d] = 0x%x\n",
+	printf("arg_int[%d] = 0x%x, result_int[%d] = 0x%x, expected_int[%d] = 0x%x\n",
 	     i, arg_int[i], i, result_int[i], i, expected_int[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -207,10 +207,10 @@ main()
   for (i = 0; i < 2; i++) {
     if (result_blli[i] != expected_blli[i])
 #ifdef DEBUG
-      printf("arg_blli[%d] = 0x%x, result_blli[%d] = 0x%llx, expected_blli[%d] = 0x%llx\n",
+	printf("arg_blli[%d] = 0x%x, result_blli[%d] = 0x%llx, expected_blli[%d] = 0x%llx\n",
 	     i, arg_blli[i], i, result_blli[i], i, expected_blli[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -224,10 +224,10 @@ main()
   for (i = 0; i < 2; i++) {
     if (result_ulli[i] != expected_ulli[i])
 #ifdef DEBUG
-      printf("arg_ulli[%d] = 0x%x, result_ulli[%d] = 0x%llx, expected_ulli[%d] = 0x%llx\n",
+	printf("arg_ulli[%d] = 0x%x, result_ulli[%d] = 0x%llx, expected_ulli[%d] = 0x%llx\n",
 	     i, arg_ulli[i], i, result_ulli[i], i, expected_ulli[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -240,10 +240,10 @@ main()
   for (i = 0; i < 2; i++) {
     if (result_lli[i] != expected_lli[i])
 #ifdef DEBUG
-      printf("arg_lli[%d] = 0x%x, result_lli[%d] = 0x%llx, expected_lli[%d] = 0x%llx\n",
+	printf("arg_lli[%d] = 0x%x, result_lli[%d] = 0x%llx, expected_lli[%d] = 0x%llx\n",
 	     i, arg_lli[i], i, result_lli[i], i, expected_lli[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 
@@ -270,7 +270,7 @@ main()
        printf("expected_uint128[0] =  %llx ", expected_uint128[0] >> 64);
        printf(" %llx\n", expected_uint128[0] & 0xFFFFFFFFFFFFFFFF);
 #else
-    abort();
+       abort();
 #endif
     }
 
@@ -296,7 +296,7 @@ main()
        printf("expected_int128[0] =  %llx ", expected_int128[0] >> 64);
        printf(" %llx\n", expected_int128[0] & 0xFFFFFFFFFFFFFFFF);
 #else
-    abort();
+       abort();
 #endif
     }
 
@@ -313,14 +313,12 @@ main()
 
   for (i = 0; i < 4; i++) {
     if (result_f[i] != expected_f[i])
-      {
 #ifdef DEBUG
 	printf("    arg_f[%d] = %f, result_f[%d] = %f, expected_f[%d] = %f\n",
-	       i, arg_f[i], i, result_f[i], i, expected_f[i]);
+	     i, arg_f[i], i, result_f[i], i, expected_f[i]);
 #else
 	abort();
 #endif
-      }
   }
 
   /* 64-bit floats */
@@ -335,10 +333,10 @@ main()
   for (i = 0; i < 2; i++) {
     if (result_d[i] != expected_d[i])
 #ifdef DEBUG
-      printf("arg_d[%d] = %f, result_d[%d] = %f, expected_d[%d] = %f\n",
+	printf("arg_d[%d] = %f, result_d[%d] = %f, expected_d[%d] = %f\n",
 	     i, arg_d[i], i, result_d[i], i, expected_d[i]);
 #else
-    abort();
+	abort();
 #endif
   }
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c b/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
index 164f11f..14ef5c7 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-xxbr-1.c
@@ -1,4 +1,4 @@
-/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-do compile { target { powerpc64*-*-*  && { lp64 && p9vector_hw } } } } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mcpu=power9 -O3" } */
@@ -10,25 +10,25 @@
 vector char
 rev_char (vector char a)
 {
-  return vec_revb (a);		/* XXBRQ.  */
+  return vec_revb (a);		/* Is a NOP, maps to move inst  */
 }
 
 vector bool char
 rev_bool_char (vector bool char a)
 {
-  return vec_revb (a);		/* XXBRQ.  */
+  return vec_revb (a);		/* Is a NOP, maps to move inst  */
 }
 
 vector signed char
 rev_schar (vector signed char a)
 {
-  return vec_revb (a);		/* XXBRQ.  */
+  return vec_revb (a);		/* Is a NOP, maps to move inst  */
 }
 
 vector unsigned char
 rev_uchar (vector unsigned char a)
 {
-  return vec_revb (a);		/* XXBRQ.  */
+  return vec_revb (a);		/* Is a NOP, maps to move inst  */
 }
 
 vector short
@@ -81,5 +81,4 @@ rev_double (vector double a)
 
 /* { dg-final { scan-assembler-times "xxbrd" 1 } } */
 /* { dg-final { scan-assembler-times "xxbrh" 3 } } */
-/* { dg-final { scan-assembler-times "xxbrq" 4 } } */
 /* { dg-final { scan-assembler-times "xxbrw" 4 } } */
-- 
2.7.4




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]