[Bug tree-optimization/64434] New: Performance regression after operand canonicalization (r216728).

[Bug tree-optimization/64434] New: Performance regression after operand canonicalization (r216728).

[ysrumyan at gmail dot com]

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

            Bug ID: 64434
           Summary: Performance regression after operand canonicalization
                    (r216728).
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ysrumyan at gmail dot com

We noticed huge regression on eembc1.1 and eembc2.0 for 32-bit target at x86.
It can be reproduced on attached test-case:

before this fix number of %esp references in assembly was 2 but after r216728 -
24. Note that simple fix resolving this issue was sent for review.

[Bug tree-optimization/64434] Performance regression after operand canonicalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #1 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
Created attachment 34345
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34345&action=edit
simple reproducer

Need to compile with -m32 on x86 platform.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-12-29
                 CC|                            |hjl.tools at gmail dot com
   Target Milestone|---                         |5.0
            Summary|Performance regression      |[5 Regression] Performance
                   |after operand               |regression after operand
                   |canonicalization (r216728). |cannibalization (r216728).
     Ever confirmed|0                           |1

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
Please show before and after assembly codes. Do we get slowdown on all
processors?

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #3 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
I put into attachment two assembly files for test-case compiled with
  "-O2 -m32 -S" options.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #4 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
Created attachment 34348
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34348&action=edit
assembly files for test.c

Assembly file fro test.c

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #5 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
Created attachment 34349
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34349&action=edit
assembly file before r216728

Assembly file.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #6 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
H.J.

I put before/after assembly files into bug attachment. We saw slowdown
on SLM and HSW for 32-bit on eembc2.0, e.g. des degradated on 36%
(SLM) and 7%(HSW). But we did not see slowdown on any 64-bit x86.

2014-12-29 18:48 GMT+03:00 hjl.tools at gmail dot com
<gcc-bugzilla@gcc.gnu.org>:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64434
>
> H.J. Lu <hjl.tools at gmail dot com> changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|NEW                         |WAITING
>
> --- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
> Please show before and after assembly codes. Do we get slowdown on all
> processors?
>
> --
> You are receiving this mail because:
> You reported the bug.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[hjl.tools at gmail dot com]

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

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> ---
r216728 generates much longer code sequences.  Where does it come from?
Does -m64 also generate longer code sequences?

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand cannibalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #8 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
The issue is caused by operand canonicalization, i.e. there is special
operand odering for commutative operations to have the same
representation for  a + b and b + a. If computation of second operand
requires more operations this may lead to live range increasing ( for
live variables computed by first operand). If we swap these operands
we get live range shrinking which is essential for 32-bit targets
having only few registers.

Hope it will help you to understand the problem.

2014-12-29 19:36 GMT+03:00 hjl.tools at gmail dot com
<gcc-bugzilla@gcc.gnu.org>:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64434
>
> --- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> ---
> r216728 generates much longer code sequences.  Where does it come from?
> Does -m64 also generate longer code sequences?
>
> --
> You are receiving this mail because:
> You reported the bug.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[5 Regression] Performance  |[5 Regression] Performance
                   |regression after operand    |regression after operand
                   |cannibalization (r216728).  |canonicalization (r216728).

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Yuri Rumyantsev from comment #8)
> The issue is caused by operand canonicalization, i.e. there is special
> operand odering for commutative operations to have the same
> representation for  a + b and b + a. If computation of second operand
> requires more operations this may lead to live range increasing ( for
> live variables computed by first operand). If we swap these operands
> we get live range shrinking which is essential for 32-bit targets
> having only few registers.
> 

Are there any benefits for operand canonicalization for x86-64? Testcases
in r216728 seems to indicate that it is a good thing to do.  There may be
a case where operand canonicalization even wins for x86.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[rguenther at suse dot de]

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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On December 29, 2014 5:56:25 PM CET, ysrumyan at gmail dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64434
>
>--- Comment #8 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
>The issue is caused by operand canonicalization, i.e. there is special
>operand odering for commutative operations to have the same
>representation for  a + b and b + a. If computation of second operand
>requires more operations this may lead to live range increasing ( for
>live variables computed by first operand). If we swap these operands
>we get live range shrinking which is essential for 32-bit targets
>having only few registers.
>
>Hope it will help you to understand the problem.

This is essentially a scheduling issue (and thus also related to TER).  I
played with the idea of a SSA life-range reducing BB scheduling without much
success.  The trick of swapping operands to expand the expensive one first
sounds good to me.

>2014-12-29 19:36 GMT+03:00 hjl.tools at gmail dot com
><gcc-bugzilla@gcc.gnu.org>:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64434
>>
>> --- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> ---
>> r216728 generates much longer code sequences.  Where does it come
>from?
>> Does -m64 also generate longer code sequences?
>>
>> --
>> You are receiving this mail because:
>> You reported the bug.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
           Priority|P3                          |P1
             Status|WAITING                     |NEW

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Shouldn't you cache the result of count_num_stmt in some SSA_NAME_VERSION
indexed vector?  Otherwise it can be expensive on pathological testcases.
And with the caching I'd hope it should be cheap enough that you could do that
even outside of loops.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[rguenther at suse dot de]

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

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 13 Jan 2015, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64434
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Shouldn't you cache the result of count_num_stmt in some SSA_NAME_VERSION
> indexed vector?  Otherwise it can be expensive on pathological testcases.
> And with the caching I'd hope it should be cheap enough that you could do that
> even outside of loops.

Yes, see my patch review

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[ienkovich at gcc dot gnu.org]

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

--- Comment #14 from ienkovich at gcc dot gnu.org ---
Author: ienkovich
Date: Thu Jan 15 11:39:20 2015
New Revision: 219646

URL: https://gcc.gnu.org/viewcvs?rev=219646&root=gcc&view=rev
Log:
gcc/

    PR tree-optimization/64434
    * cfgexpand.c (reorder_operands): New function.
    (expand_gimple_basic_block): Insert call of reorder_operands if
    optimized is true.

gcc/testsuite/

    PR tree-optimization/64434
    * gcc.dg/torture/pr64434.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr64434.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgexpand.c
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[rguenth at gcc dot gnu.org]

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

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.  Can you please move the testcase away from the torture (you specify
-O1...)?  Just to gcc.dg/

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[ienkovich at gcc dot gnu.org]

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

--- Comment #17 from ienkovich at gcc dot gnu.org ---
Author: ienkovich
Date: Fri Jan 16 14:22:57 2015
New Revision: 219741

URL: https://gcc.gnu.org/viewcvs?rev=219741&root=gcc&view=rev
Log:
gcc/testsuite/

    PR tree-optimization/64434
    * gcc.dg/torture/pr64434.c: Move to...
    * gcc.dg/pr64434.c: ... here.


Added:
    trunk/gcc/testsuite/gcc.dg/pr64434.c
      - copied unchanged from r219740,
trunk/gcc/testsuite/gcc.dg/torture/pr64434.c
Removed:
    trunk/gcc/testsuite/gcc.dg/torture/pr64434.c
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[pinskia at gcc dot gnu.org]

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

--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This testcase fails for AARCH64:ilp32 (that is AARCH64 with -mabi=ilp32).

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #19 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
Andrew!

Could you please try modified test-case (test1.c) which is attached.

Thanks.

[Bug tree-optimization/64434] [5 Regression] Performance regression after operand canonicalization (r216728).

[ysrumyan at gmail dot com]

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

--- Comment #20 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
Created attachment 34700
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34700&action=edit
another test-case