[Bug tree-optimization/67945] New: Testsuite failures starting with revision 228616

[Bug tree-optimization/67945] New: Testsuite failures starting with revision 228616

[pthaugen at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

            Bug ID: 67945
           Summary: Testsuite failures starting with revision 228616
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pthaugen at gcc dot gnu.org
                CC: bergner at gcc dot gnu.org, dje at gcc dot gnu.org,
                    rdsandiford at googlemail dot com
  Target Milestone: ---
              Host: powerpc64-unknown-linux-gnu
            Target: powerpc64-unknown-linux-gnu
             Build: powerpc64-unknown-linux-gnu

Following tests started failing with r228616.

FAIL: gcc.target/powerpc/ppc-pow.c scan-assembler-times bl?[\\\\. ]+pow 1
FAIL: gcc.target/powerpc/ppc-pow.c scan-assembler-times fsqrt 3
FAIL: gcc.target/powerpc/pr46728-2.c scan-assembler-times fsqrt|xssqrtdp 4
FAIL: gcc.target/powerpc/pr46728-3.c scan-assembler-times sqrt 4
FAIL: gcc.target/powerpc/pr46728-5.c scan-assembler-times cbrt 2

[Bug tree-optimization/67945] Testsuite failures starting with revision 228616

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

Thomas Preud'homme <thopre01 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thopre01 at gcc dot gnu.org

--- Comment #1 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
arm-none-eabi target also regressed on:

PASS->FAIL: gcc.dg/builtins-10.c (test for excess errors)

This is due to sqrt(pow(x,4.0)) != x*x not being folded correctly (other tests
are fine).

[Bug tree-optimization/67945] Testsuite failures starting with revision 228616

[afomin.mailbox at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

Alexander Fomin <afomin.mailbox at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |afomin.mailbox at gmail dot com

--- Comment #2 from Alexander Fomin <afomin.mailbox at gmail dot com> ---
At least x86-64 also regressed ont SPEC2006 410.bwaves because of the same
folding problem.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |6.0
            Summary|Testsuite failures starting |[6 Regression] Testsuite
                   |with revision 228616        |failures starting with
                   |                            |revision 228616

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Was this maybe fixed by

2015-10-13  Richard Sandiford  <richard.sandiford@arm.com>

        * real.h (real_isinteger): Declare.
        * real.c (real_isinteger): New function.
        * match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
        if y is an even integer.

?

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #4 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> Was this maybe fixed by
> 
> 2015-10-13  Richard Sandiford  <richard.sandiford@arm.com>
> 
>         * real.h (real_isinteger): Declare.
>         * real.c (real_isinteger): New function.
>         * match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
>         if y is an even integer.
> 
> ?

It does for builtins-10.c on arm-none-eabi at least. Thanks Richard S for the
quick fix.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[afomin.mailbox at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #5 from Alexander Fomin <afomin.mailbox at gmail dot com> ---
Created attachment 36500
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36500&action=edit
Folding regression reproducer

Unfortunately, it does not for x86-64.
Please take a look at the attached reproducer.
When compiling with -m64 -O2 -ffast-math -S it does not perform expected 
folding.

r228614
<bb 2>:
y.1_2 = y;
_3 = __builtin_sqrt (y.1_2);
*x_4(D) = _3;
powroot_18 = __builtin_sqrt (_3);
powroot_19 = _3 * powroot_18;
powroot_20 = y.1_2 * powroot_19;
y = powroot_20;

r228616
<bb 2>:
y.1_2 = y;
_3 = __builtin_sqrt (y.1_2);
*x_4(D) = _3;
powroot_18 = __builtin_pow (y.1_2, 2.5e-1);
powroot_19 = _3 * powroot_18;
powroot_20 = y.1_2 * powroot_19;
y = powroot_20;

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[pthaugen at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #6 from Pat Haugen <pthaugen at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> Was this maybe fixed by
> 
> 2015-10-13  Richard Sandiford  <richard.sandiford@arm.com>
> 
>         * real.h (real_isinteger): Declare.
>         * real.c (real_isinteger): New function.
>         * match.pd: Simplify pow(|x|,y) and pow(-x,y) to pow(x,y)
>         if y is an even integer.
> 
> ?

That did not fix the powerpc64 failures, they still fail with r228762.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-10-13
                 CC|                            |rsandifo at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Sorry, looks like it was a mistake to think that this transition
could be done one function at a time.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Actually, the problem seems to be that we're oscillating
between forms.

The old fold-const.c folders converted sequences of sqrts
and cbrts into a pow call while tree-ssa-math-opts.c goes
the other way.  In that situation things were well-controlled:
we did both transformations only once, in that order.
Moving the fold-const.c folders to match.pd means that we
try them many times and that ultimately they win.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #8)
> Actually, the problem seems to be that we're oscillating
> between forms.
> 
> The old fold-const.c folders converted sequences of sqrts
> and cbrts into a pow call while tree-ssa-math-opts.c goes
> the other way.  In that situation things were well-controlled:
> we did both transformations only once, in that order.
> Moving the fold-const.c folders to match.pd means that we
> try them many times and that ultimately they win.

Ah, I was worried about such cases.  The fold-const.c/builtins.c code
has two kinds of transforms - canonicalizations (producing for example
pow (x, 2) from x*x or like in this case pow from sqrt*cbrt) and
"simplifications" (well, transforms to "faster" implementations).
The canonicalizations are purely to catch more CSE and simplifications.

I wonder if we should have two "phases" of simplifications, first
aggressively canonicalize and later apply the optimizations.  We could
key this on a new GIMPLE state, PROP_gimple_XXX (can't figure out a good
acronym) and key patterns with it (like we have reload_completed on RTL).
The pass deciding it's time to "lower" stuff (rather than canonicalize)
would be cse_sincos (the pass doing the transform required
for the testcase).  Note that some patters in that pass might be moved
to match.pd instead so we can key PROP_gimple_XXX on the forwprop pass
preceeding cse_sincos (so we can transform all stmts at once).

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #9)
> I wonder if we should have two "phases" of simplifications, first
> aggressively canonicalize and later apply the optimizations.  We could
> key this on a new GIMPLE state, PROP_gimple_XXX (can't figure out a good
> acronym) and key patterns with it (like we have reload_completed on RTL).
> The pass deciding it's time to "lower" stuff (rather than canonicalize)
> would be cse_sincos (the pass doing the transform required
> for the testcase).  Note that some patters in that pass might be moved
> to match.pd instead so we can key PROP_gimple_XXX on the forwprop pass
> preceeding cse_sincos (so we can transform all stmts at once).

OK, thanks for the pointer.  I'm giving that a go now.

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Author: rsandifo
Date: Thu Oct 15 09:50:07 2015
New Revision: 228840

URL: https://gcc.gnu.org/viewcvs?rev=228840&root=gcc&view=rev
Log:
PR67945: Fix oscillation between pow representations

This patch fixes some fallout from my patch to move the sqrt and cbrt
folding rules to match.pd.  The rules included canonicalisations like:

       sqrt(sqrt(x))->pow(x,1/4)

which in the original code was only ever done at the generic level.
My patch meant that we'd do it whenever we tried to fold a gimple
statement, and eventually it would win over the sincos optimisation
that replaces pow(x,1/4) with sqrt(sqrt(x)).

Following a suggestion from Richard B, the patch adds a new
PROP_gimple_* flag to say whether fp routines have been optimised
for the target.  If so, match.pd should only transform calls to math
functions if the result is actually an optimisation, not just an
IL simplification or canonicalisation.  The question then of course
is: which rules are which?  I've added block comments that describe
the criteria I was using.

A slight wart is that we need to use the cfun global to access
the PROP_gimple_* flag; there's no local function pointer available.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested
on powerc64-linux-gnu.

gcc/
        PR tree-optimization/67945
        * tree-pass.h (PROP_gimple_opt_math): New property flag.
        * generic-match-head.c (canonicalize_math_p): New function.
        * gimple-match-head.c: Include tree-pass.h.
        (canonicalize_math_p): New function.
        * match.pd: Group math built-in rules into simplifications
        and canonicalizations.  Guard the latter with canonicalize_math_p.
        * tree-ssa-math-opts.c (pass_data_cse_sincos): Provide the
        PROP_gimple_opt_math property.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/generic-match-head.c
    trunk/gcc/gimple-match-head.c
    trunk/gcc/match.pd
    trunk/gcc/tree-pass.h
    trunk/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/67945] [6 Regression] Testsuite failures starting with revision 228616

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67945

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed.  Sorry for the breakage.