| Message ID | 1411562769.2935.42.camel@gnopaine |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 24, 2014 at 8:46 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > Hi, > > The analyze_swaps pass performs special handling on certain non-swapping > loads and stores so that computations involving them can still be > optimized. However, the intent was to avoid this for lvx, stvx, lve*, > and stve*. The existing logic excludes these by looking for a PARALLEL > as the rtx code for the insn body. It turns out this works for lvx, > stvx, and lve*, but stve* was implemented slightly differently, so this > check doesn't catch it. This patch fixes the problem by looking for the > pattern that matches stve* as well; we now exclude any store with an > UNSPEC as its SET_SRC. > > I've added a new compile-time test case to verify the fix. The test > ICEs on existing trunk but passes with the new changes. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no > regressions. Is this ok for trunk? > > Thanks, > Bill > > > [gcc] > > 2014-09-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > * config/rs6000/rs6000.c (insn_is_swappable_p): Don't provide > special handling for stores whose SET_SRC is an UNSPEC (such as > UNSPEC_STVE). > > > [gcc/testsuite] > > 2014-09-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > * gcc.target/powerpc/swaps-p8-17.c: New test. Okay. Thanks, David
Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 215486) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -33793,9 +33793,10 @@ insn_is_swappable_p (swap_web_entry *insn_entry, r return 0; /* Loads and stores seen here are not permuting, but we can still - fix them up by converting them to permuting ones. Exception: - UNSPEC_LVX and UNSPEC_STVX, which have a PARALLEL body instead - of a SET. */ + fix them up by converting them to permuting ones. Exceptions: + UNSPEC_LVE, UNSPEC_LVX, and UNSPEC_STVX, which have a PARALLEL + body instead of a SET; and UNSPEC_STVE, which has an UNSPEC + for the SET source. */ rtx body = PATTERN (insn); int i = INSN_UID (insn); @@ -33812,7 +33813,7 @@ insn_is_swappable_p (swap_web_entry *insn_entry, r if (insn_entry[i].is_store) { - if (GET_CODE (body) == SET) + if (GET_CODE (body) == SET && GET_CODE (SET_SRC (body)) != UNSPEC) { *special = SH_NOSWAP_ST; return 1; Index: gcc/testsuite/gcc.target/powerpc/swaps-p8-17.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/swaps-p8-17.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/swaps-p8-17.c (working copy) @@ -0,0 +1,14 @@ +/* { dg-do compile { target { powerpc64le-*-* } } } */ +/* { dg-options "-mcpu=power8 -O1" } */ +/* { dg-final { scan-assembler "lxvd2x" } } */ +/* { dg-final { scan-assembler "xxpermdi" } } */ + +/* Verify that we don't try to do permute removal in the presence of + vec_ste. This used to ICE. */ +#include <altivec.h> + +void f (void *p) +{ + vector unsigned int u32 = vec_vsx_ld (1, (const unsigned int *)p); + vec_ste (u32, 1, (unsigned int *)p); +}