diff mbox

[rs6000] Fix PR79044 (ICE in swap optimization)

Message ID a5f8df9e-da38-0807-bf51-0958bab40bb3@linux.vnet.ibm.com
State New
Headers show

Commit Message

Bill Schmidt Jan. 10, 2017, 8:18 p.m. UTC 
Hi,

PR79044 reports a situation where swap optimization ICEs in GCC 6 and in trunk.  The
problem is that swap optimization doesn't properly recognize that element-reversing
loads and stores (e.g., lxvw4x) cannot be treated as "swappable" instructions.  These
arise from the __builtin_vec_xl and __builtin_vec_xst interfaces that were added in 
GCC 6.  The surrounding code is slightly different, so the fix is slightly different
for the two releases.

The fix is obvious, and bootstraps on powerpc64le-unknown-linux-gnu with no regressions.
Are these patches ok for trunk and GCC 6, respectively?

Thanks,
Bill


For current trunk:

[gcc]

2017-01-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/79044
	* config/rs6000/rs6000.c (insn_is_swappable_p): Mark
	element-reversing loads and stores as not swappable.

[gcc/testsuite]

2017-01-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/79044
	* gcc.target/powerpc/swaps-p8-26.c: New.

Comments

Segher Boessenkool Jan. 11, 2017, 11:36 p.m. UTC | #1 
On Tue, Jan 10, 2017 at 02:18:38PM -0600, Bill Schmidt wrote:
> PR79044 reports a situation where swap optimization ICEs in GCC 6 and in trunk.  The
> problem is that swap optimization doesn't properly recognize that element-reversing
> loads and stores (e.g., lxvw4x) cannot be treated as "swappable" instructions.  These
> arise from the __builtin_vec_xl and __builtin_vec_xst interfaces that were added in 
> GCC 6.  The surrounding code is slightly different, so the fix is slightly different
> for the two releases.
> 
> The fix is obvious, and bootstraps on powerpc64le-unknown-linux-gnu with no regressions.
> Are these patches ok for trunk and GCC 6, respectively?

Okay for both.  Is this needed for GCC 5 as well?

Thanks,


Segher
Bill Schmidt Jan. 12, 2017, 2:12 p.m. UTC | #2 
> On Jan 11, 2017, at 5:36 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Tue, Jan 10, 2017 at 02:18:38PM -0600, Bill Schmidt wrote:
>> PR79044 reports a situation where swap optimization ICEs in GCC 6 and in trunk.  The
>> problem is that swap optimization doesn't properly recognize that element-reversing
>> loads and stores (e.g., lxvw4x) cannot be treated as "swappable" instructions.  These
>> arise from the __builtin_vec_xl and __builtin_vec_xst interfaces that were added in 
>> GCC 6.  The surrounding code is slightly different, so the fix is slightly different
>> for the two releases.
>> 
>> The fix is obvious, and bootstraps on powerpc64le-unknown-linux-gnu with no regressions.
>> Are these patches ok for trunk and GCC 6, respectively?
> 
> Okay for both.  Is this needed for GCC 5 as well?

No, the potential for this problem was introduced in 6.  Thanks for the review!

Bill

> 
> Thanks,
> 
> 
> Segher
>
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 244274)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -41344,7 +41344,10 @@  insn_is_swappable_p (swap_web_entry *insn_entry, r
       if (GET_CODE (body) == SET)
 	{
 	  rtx rhs = SET_SRC (body);
-	  gcc_assert (GET_CODE (rhs) == MEM);
+	  /* Even without a swap, the RHS might be a vec_select for, say,
+	     a byte-reversing load.  */
+	  if (GET_CODE (rhs) != MEM)
+	    return 0;
 	  if (GET_CODE (XEXP (rhs, 0)) == AND)
 	    return 0;
 
@@ -41361,7 +41364,10 @@  insn_is_swappable_p (swap_web_entry *insn_entry, r
 	  && GET_CODE (SET_SRC (body)) != UNSPEC)
 	{
 	  rtx lhs = SET_DEST (body);
-	  gcc_assert (GET_CODE (lhs) == MEM);
+	  /* Even without a swap, the LHS might be a vec_select for, say,
+	     a byte-reversing store.  */
+	  if (GET_CODE (lhs) != MEM)
+	    return 0;
 	  if (GET_CODE (XEXP (lhs, 0)) == AND)
 	    return 0;
 	  
Index: gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c	(working copy)
@@ -0,0 +1,21 @@ 
+/* { dg-do compile { target { powerpc64le-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O3 " } */
+/* { dg-final { scan-assembler-times "lxvw4x" 2 } } */
+/* { dg-final { scan-assembler "stxvw4x" } } */
+/* { dg-final { scan-assembler-not "xxpermdi" } } */
+
+/* Verify that swap optimization does not interfere with element-reversing
+   loads and stores.  */
+
+/* Test case to resolve PR79044.  */
+
+#include <altivec.h>
+
+void pr79044 (float *x, float *y, float *z)
+{
+  vector float a = __builtin_vec_xl (0, x);
+  vector float b = __builtin_vec_xl (0, y);
+  vector float c = __builtin_vec_mul (a, b);
+  __builtin_vec_xst (c, 0, z);
+}



For GCC 6:


[gcc]

2017-01-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/79044
	* config/rs6000/rs6000.c (insn_is_swappable_p): Mark
	element-reversing loads and stores as not swappable.

[gcc/testsuite]

2017-01-10  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/79044
	* gcc.target/powerpc/swaps-p8-26.c: New.


Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 244276)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -38657,6 +38657,12 @@  insn_is_swappable_p (swap_web_entry *insn_entry, r
     {
       if (GET_CODE (body) == SET)
 	{
+	  rtx rhs = SET_SRC (body);
+	  /* Even without a swap, the RHS might be a vec_select for, say,
+	     a byte-reversing load.  */
+	  if (GET_CODE (rhs) != MEM)
+	    return 0;
+
 	  *special = SH_NOSWAP_LD;
 	  return 1;
 	}
@@ -38668,6 +38674,12 @@  insn_is_swappable_p (swap_web_entry *insn_entry, r
     {
       if (GET_CODE (body) == SET && GET_CODE (SET_SRC (body)) != UNSPEC)
 	{
+	  rtx lhs = SET_DEST (body);
+	  /* Even without a swap, the LHS might be a vec_select for, say,
+	     a byte-reversing store.  */
+	  if (GET_CODE (lhs) != MEM)
+	    return 0;
+
 	  *special = SH_NOSWAP_ST;
 	  return 1;
 	}
Index: gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/swaps-p8-26.c	(working copy)
@@ -0,0 +1,21 @@ 
+/* { dg-do compile { target { powerpc64le-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O3 " } */
+/* { dg-final { scan-assembler-times "lxvw4x" 2 } } */
+/* { dg-final { scan-assembler "stxvw4x" } } */
+/* { dg-final { scan-assembler-not "xxpermdi" } } */
+
+/* Verify that swap optimization does not interfere with element-reversing
+   loads and stores.  */
+
+/* Test case to resolve PR79044.  */
+
+#include <altivec.h>
+
+void pr79044 (float *x, float *y, float *z)
+{
+  vector float a = __builtin_vec_xl (0, x);
+  vector float b = __builtin_vec_xl (0, y);
+  vector float c = __builtin_vec_mul (a, b);
+  __builtin_vec_xst (c, 0, z);
+}