Re: [PATCH, rs6000] Fix PR85755: PowerPC Gcc's -mupdate produces inefficient code

[Segher Boessenkool <segher at kernel dot crashing dot org>]

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