Re: [PATCH, rs6000] Fix PR79044 (ICE in swap optimization)

[Bill Schmidt]

> On Jan 11, 2017, at 5:36 PM, Segher Boessenkool  
> 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
> 

Re: [PATCH, rs6000] Fix PR79044 (ICE in swap optimization)

[Segher Boessenkool]

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

[PATCH, rs6000] Fix PR79044 (ICE in swap optimization)

[Bill Schmidt]

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  

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  

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


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 
+
+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  

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  

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" } } */
+/* {