[Bug target/57293] New: [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[Bug target/57293] New: [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[fanael4 at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

            Bug ID: 57293
           Summary: [4.8/4.9 Regression] not needed frame pointers on
                    IA-32 (performance regression?)
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fanael4 at gmail dot com

About a week ago GCC started to emit frame pointers even in presence of
-fomit-frame-pointer whenever there's a call to a function with a "callee
cleans up the arguments" calling convention. Normally it wouldn't be a big
deal, but IA-32 is seriously register-starved, and since one of the secondary
platforms (i.e. Windows) uses __thiscall__ for C++ member functions by default,
for some code the availability of ebp can make a noticeable difference. I can
provide a benchmark if one is needed.

The culprit seems to be revisions 198140 and 198555.

Testcase:

/* compile with -O2 -fomit-frame-pointer */
__attribute__((__noinline__, __noclone__, __stdcall__)) void g(int a)
{
  __builtin_printf("in g(): %d\n", a);
}

__attribute__((__noinline__, __noclone__, __thiscall__)) void h(int a, int b)
{
  __builtin_printf("in h(): %d %d\n", a, b);
}

void f()
{
  g(0);
  h(0, 1);
  __builtin_puts("in f()");
}

What GCC 4.7.0 and 4.8.1 20130430 produce for f :

_f:
LFB2:
        .cfi_startproc
        subl    $28, %esp
        .cfi_def_cfa_offset 32
        movl    $0, (%esp)
        call    _g@4
        .cfi_def_cfa_offset 28
        xorl    %ecx, %ecx
        subl    $4, %esp
        .cfi_def_cfa_offset 32
        movl    $1, (%esp)
        call    _h
        .cfi_def_cfa_offset 28
        subl    $4, %esp
        .cfi_def_cfa_offset 32
        movl    $LC2, (%esp)
        call    _puts
        addl    $28, %esp
        .cfi_def_cfa_offset 4
        ret
        .cfi_endproc

What 4.8.1 20130510 produces:

_f:
LFB2:
        .cfi_startproc
        pushl   %ebp
        .cfi_def_cfa_offset 8
        .cfi_offset 5, -8
        movl    %esp, %ebp
        .cfi_def_cfa_register 5
        subl    $24, %esp
        movl    $0, (%esp)
        call    _g@4
        xorl    %ecx, %ecx
        subl    $4, %esp
        movl    $1, (%esp)
        call    _h
        subl    $4, %esp
        movl    $LC2, (%esp)
        call    _puts
        leave
        .cfi_restore 5
        .cfi_def_cfa 4, 4
        ret
        .cfi_endproc

Target: i686-w64-mingw32
Configured with: ../../src/gcc-svn/configure --build=x86_64-unknown-linux-gnu
--host=i686-w64-mingw32 --target=i686-w64-mingw32 --disable-multilib
--disable-multiarch --disable-nls --enable-languages=c,c++,lto
--disable-win32-registry --enable-openmp --enable-libgomp
--enable-threads=posix --enable-plugins --enable-static --enable-shared
--disable-symvers --enable-fully-dynamic-string --disable-sjlj-exceptions
--disable-libstdcxx-pch --enable-libstdcxx-time --with-arch=i686
--enable-checking=release --disable-werror --with-gnu-as --with-gnu-ld
--disable-rpath --with-gmp=/mingw/i686-final/libs-out-dir
--with-mpfr=/mingw/i686-final/libs-out-dir
--with-mpc=/mingw/i686-final/libs-out-dir
--with-isl=/mingw/i686-final/libs-out-dir
--with-cloog=/mingw/i686-final/libs-out-dir
--with-libiconv-prefix=/mingw/i686-final/libs-out-dir --with-system-zlib
--prefix=/mingw/i686-final/out-dir
Thread model: posix
gcc version 4.8.1 20130510 (prerelease) (GCC)

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Target|                            |i686-w64-mingw32
   Target Milestone|---                         |4.8.1

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[vmakarov at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

Vladimir Makarov <vmakarov at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vmakarov at redhat dot com

--- Comment #1 from Vladimir Makarov <vmakarov at redhat dot com> ---
The change was done because of 

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57018

LRA misses some functionality for now for this kind of code.  There will be no
quick fix (I mean in a few days or even in 2 weeks) for this.  But I am
planning to fix it until end of June.

Sorry.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-05-16
   Target Milestone|4.8.1                       |4.8.2
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Thanks, postponing the fix for 4.8.2 then.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[fanael4 at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

Fanael <fanael4 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|i686-w64-mingw32            |i?86-*-*

--- Comment #3 from Fanael <fanael4 at gmail dot com> ---
Reproduced on x86_64-unknown-linux-gnu (*) with -m32, thus not related to MinGW
itself.

(*) $ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-unknown-linux-gnu/4.8.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: /build/gcc-multilib/src/gcc-4.8-20130502/configure
--prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/
--enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared
--enable-threads=posix --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch
--enable-gnu-unique-object --enable-linker-build-id --enable-cloog-backend=isl
--disable-cloog-version-check --enable-lto --enable-gold --enable-ld=default
--enable-plugin --with-plugin-ld=ld.gold --with-linker-hash-style=gnu
--disable-install-libiberty --enable-multilib --disable-libssp --disable-werror
--enable-checking=release
Thread model: posix
gcc version 4.8.0 20130502 (prerelease) (GCC)

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[fanael4 at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

--- Comment #4 from Fanael <fanael4 at gmail dot com> ---
(In reply to Vladimir Makarov from comment #1)
> But I am planning to fix it until end of June.

Any progress on this one? Patching GCC to use Satan^H^H^H^H^Hreload is a
workaround, but one I'd rather avoid if at all possible.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[vmakarov at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

--- Comment #5 from Vladimir Makarov <vmakarov at redhat dot com> ---
I've started this work.  But unfortunately, i have too many things on my plate
now.  I was too optimistic.  Now I can say only that I am planning to fix it on
stage1 (so the fix should be in gcc4.9).

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.2                       |4.8.3

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.2 has been released.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ra
           Priority|P3                          |P2

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[vmakarov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

--- Comment #7 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
The problem was fixed by a patch removing regmove and improving hardware
preferences in IRA.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[vmakarov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

--- Comment #8 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
Sorry, ignore my previous comment.  It should be for PR56339.

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[vmakarov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

--- Comment #9 from Vladimir Makarov <vmakarov at gcc dot gnu.org> ---
Author: vmakarov
Date: Thu Nov 28 21:45:21 2013
New Revision: 205498

URL: http://gcc.gnu.org/viewcvs?rev=205498&root=gcc&view=rev
Log:
2013-11-28  Vladimir Makarov  <vmakarov@redhat.com>

    PR target/57293
    * ira.h (ira_setup_eliminable_regset): Remove parameter.
    * ira.c (ira_setup_eliminable_regset): Ditto.  Add
    SUPPORTS_STACK_ALIGNMENT for crtl->stack_realign_needed.
    Don't call lra_init_elimination.
    (ira): Call ira_setup_eliminable_regset without arguments.
    * loop-invariant.c (calculate_loop_reg_pressure): Remove argument
    from ira_setup_eliminable_regset call.
    * gcse.c (calculate_bb_reg_pressure): Ditto.
    * haifa-sched.c (sched_init): Ditto.
    * lra.h (lra_init_elimination): Remove the prototype.
    * lra-int.h (lra_insn_recog_data): New member sp_offset.  Move
    used_insn_alternative upper.
    (lra_eliminate_regs_1): Add one more parameter.
    (lra-eliminate): Ditto.
    * lra.c (lra_invalidate_insn_data): Set sp_offset.
    (setup_sp_offset): New.
    (lra_process_new_insns): Call setup_sp_offset.
    (lra): Add argument to lra_eliminate calls.
    * lra-constraints.c (get_equiv_substitution): Rename to get_equiv.
    (get_equiv_with_elimination): New.
    (process_addr_reg): Call get_equiv_with_elimination instead of
    get_equiv_substitution.
    (equiv_address_substitution): Ditto.
    (loc_equivalence_change_p): Ditto.
    (loc_equivalence_callback, lra_constraints): Ditto.
    (curr_insn_transform): Ditto.  Print the sp offset
    (process_alt_operands): Prevent stack pointer reloads.
    (lra_constraints): Remove one argument from lra_eliminate call.
    Move it up.  Mark used hard regs bfore it.  Use
    get_equiv_with_elimination instead of get_equiv_substitution.
    * lra-eliminations.c (lra_eliminate_regs_1): Add parameter and
    assert for param values combination.  Use sp offset.  Add argument
    to lra_eliminate_regs_1 calls.
    (lra_eliminate_regs): Add argument to lra_eliminate_regs_1 call.
    (curr_sp_change): New static var.
    (mark_not_eliminable): Add parameter.  Update curr_sp_change.
    Don't prevent elimination to sp if we can calculate its change.
    Pass the argument to mark_not_eliminable calls.
    (eliminate_regs_in_insn): Add a parameter.  Use sp offset.  Add
    argument to lra_eliminate_regs_1 call.
    (update_reg_eliminate): Move calculation of hard regs for spill
    lower.  Switch off lra_in_progress temporarily to generate regs
    involved into elimination.
    (lra_init_elimination): Rename to init_elimination.  Make it
    static.  Set up insn sp offset, check the offsets at the end of
    BBs.
    (process_insn_for_elimination): Add parameter.  Pass its value to
    eliminate_regs_in_insn.
    (lra_eliminate): : Add parameter.  Pass its value to
    process_insn_for_elimination.  Add assert for param values
    combination.  Call init_elimination.  Don't update offsets in
    equivalence substitutions.
    * lra-spills.c (assign_mem_slot): Don't call lra_eliminate_regs_1
    for created stack slot.
    (remove_pseudos): Call lra_eliminate_regs_1 before changing memory
    onto stack slot.

2013-11-28  Vladimir Makarov  <vmakarov@redhat.com>

    PR target/57293
    * gcc.target/i386/pr57293.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr57293.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gcse.c
    trunk/gcc/haifa-sched.c
    trunk/gcc/ira.c
    trunk/gcc/ira.h
    trunk/gcc/loop-invariant.c
    trunk/gcc/lra-constraints.c
    trunk/gcc/lra-eliminations.c
    trunk/gcc/lra-int.h
    trunk/gcc/lra-spills.c
    trunk/gcc/lra.c
    trunk/gcc/lra.h
    trunk/gcc/testsuite/ChangeLog

[Bug target/57293] [4.8/4.9 Regression] not needed frame pointers on IA-32 (performance regression?)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57293

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |law at redhat dot com
         Resolution|---                         |FIXED

--- Comment #10 from Jeffrey A. Law <law at redhat dot com> ---
Vlad fixed this a couple months ago on the trunk.