On Fri, Jun 08, 2018 at 10:35:22AM -0500, Peter Bergner wrote:
> The fix for PR83969 accidentally disallowed PRE_INC and PRE_DEC addresses
> from being matched for the Y constraint leading to poor code generation.
> The old PRE_INC and PRE_DEC addresses were originally accepted via the
> address_offset() call and test, but the fix for PR83969 added the test
> for rs6000_offsettable_memref_p() and that doesn't accept PRE_INC/PRE_DEC.
>
> My earlier patch just tried moving the rs6000_offsettable_memref_p() call
> to after the address_offset() call, but I now remember why I had it placed
> before it. The problem was that the address_offset() call and test was
> incorrectly accepting some non-offsetable addresses, so we need to test for
> rs6000_offsettable_memref_p() first. However, rs6000_offsettable_memref_p()
> doesn't accept PRE_INC/PRE_DEC addresses, so the fix used here is to just
> test for them explicitly before the other tests, which fixes the reported
> bug and doesn't regress the older bugs.
>
> Is this ok for trunk and after some trunk burn in, the GCC 8 and 7 release
> branches where the earlier fixes were backported to? All bootstrap builds
> completed and the testsuite runs all showed no regressions.
Looks good, please commit. Modulo some testcase things:
> --- gcc/testsuite/gcc.target/powerpc/pr85755.c (nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr85755.c (working copy)
> @@ -0,0 +1,24 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> "-mcpu=power8" } } */
> +/* { dg-options "-O1 -mcpu=power8" } */
> +
> +void
> +preinc (long *q, long n)
> +{
> + long i;
> + for (i = 0; i < n; i++)
> + q[i] = i;
> +}
> +
> +void
> +predec (long *q, long n)
> +{
> + long i;
> + for (i = n; i >= 0; i--)
> + q[i] = i;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mstdu\M} 2 } } */
> +/* { dg-final { scan-assembler-not {\mstfdu\M} } } */
Does this need p8 at all? Would it be better to just test without -mcpu=,
on just whatever default cpu is thrown at it? p8 is default for powerpc64le
so it will get plenty coverage.
You do need an lp64 target btw.
Thanks!
Segher
