[Bug middle-end/47735] New: [4.5/4.6 Regression] Unnecessary adjustments to stack pointer

[Bug middle-end/47735] New: [4.5/4.6 Regression] Unnecessary adjustments to stack pointer

[hjl.tools at gmail dot com]

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

           Summary: [4.5/4.6 Regression] Unnecessary adjustments to stack
                    pointer
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com
                CC: matz@gcc.gnu.org


For this tescase, gcc 4.4 generates the most efficient code
while 4.5/4.6 become worse and worse:

[hjl@gnu-6 gcc]$ cat x.i 
unsigned mulh(unsigned a, unsigned b)
{
  unsigned long long l __attribute__ ((aligned(32)))
    =((unsigned long long)a * (unsigned long long)b) >> 32;
  return l;
}
[hjl@gnu-6 gcc]$ /usr/gcc-4.4/bin/gcc -O2 -S -fomit-frame-pointer x.i -m32
[hjl@gnu-6 gcc]$ cat x.s 
    .file    "x.i"
    .text
    .p2align 4,,15
.globl mulh
    .type    mulh, @function
mulh:
    movl    8(%esp), %eax
    mull    4(%esp)
    movl    %edx, %eax
    ret
    .size    mulh, .-mulh
    .ident    "GCC: (GNU) 4.4.4"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 gcc]$ /usr/gcc-4.5/bin/gcc -O2 -S -fomit-frame-pointer x.i -m32
[hjl@gnu-6 gcc]$ cat x.s
    .file    "x.i"
    .text
    .p2align 4,,15
.globl mulh
    .type    mulh, @function
mulh:
    pushl    %ebp
    movl    %esp, %ebp
    movl    12(%ebp), %eax
    mull    8(%ebp)
    popl    %ebp
    movl    %edx, %eax
    ret
    .size    mulh, .-mulh
    .ident    "GCC: (GNU) 4.5.1 20100507 (prerelease) [gcc-4_5-branch revision
159167]"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 gcc]$ ./xgcc -B./  -O2 -S -fomit-frame-pointer x.i -m32
[hjl@gnu-6 gcc]$ cat x.s
    .file    "x.i"
    .text
    .p2align 4,,15
    .globl    mulh
    .type    mulh, @function
mulh:
.LFB0:
    .cfi_startproc
    pushl    %ebp
    .cfi_def_cfa_offset 8
    .cfi_offset 5, -8
    movl    %esp, %ebp
    .cfi_def_cfa_register 5
    andl    $-32, %esp
    movl    12(%ebp), %eax
    mull    8(%ebp)
    leave
    .cfi_restore 5
    .cfi_def_cfa 4, 4
    movl    %edx, %eax
    ret
    .cfi_endproc
.LFE0:
    .size    mulh, .-mulh
    .ident    "GCC: (GNU) 4.6.0 20110131 (experimental)"
    .section    .note.GNU-stack,"",@progbits
[hjl@gnu-6 gcc]$

This is caused by revision 146817:

http://gcc.gnu.org/ml/gcc-cvs/2009-04/msg01459.html

[Bug middle-end/47735] [4.5/4.6 Regression] Unnecessary adjustments to stack pointer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.5.3

[Bug middle-end/47735] [4.5/4.6 Regression] Unnecessary adjustments to stack pointer

[steven at gcc dot gnu.org]

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

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.02.20 15:24:07
                 CC|                            |steven at gcc dot gnu.org
     Ever Confirmed|0                           |1

[Bug middle-end/47735] [4.5/4.6 Regression] Unnecessary adjustments to stack pointer

[jakub at gcc dot gnu.org]

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

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

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

[Bug middle-end/47735] [4.5/4.6/4.7 Regression] Unnecessary adjustments to stack pointer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.3                       |4.5.4

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-28 14:51:27 UTC ---
GCC 4.5.3 is being released, adjusting target milestone.

[Bug middle-end/47735] [4.5/4.6/4.7/4.8 Regression] Unnecessary adjustments to stack pointer

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.4                       |4.6.4

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 11:22:04 UTC ---
The 4.5 branch is being closed, adjusting target milestone.

[Bug middle-end/47735] [4.7/4.8/4.9 Regression] Unnecessary adjustments to stack pointer

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.4                       |4.7.4

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:16:49 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug middle-end/47735] [4.7/4.8/4.9 Regression] Unnecessary adjustments to stack pointer

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I wonder what is the point of even looking at the alignment of VAR_DECLs that
are SSA_NAME_VAR of SSA_NAMEs if we're not putting those into stack.

So perhaps something like:
--- gcc/cfgexpand.c.jj    2013-12-16 09:08:17.000000000 +0100
+++ gcc/cfgexpand.c    2014-01-02 10:04:39.525480578 +0100
@@ -1215,8 +1215,11 @@ expand_one_var (tree var, bool toplevel,
      we conservatively assume it will be on stack even if VAR is
      eventually put into register after RA pass.  For non-automatic
      variables, which won't be on stack, we collect alignment of
-     type and ignore user specified alignment.  */
-      if (TREE_STATIC (var) || DECL_EXTERNAL (var))
+     type and ignore user specified alignment.  Similarly for
+     SSA_NAMEs for which use_register_for_decl returns true.  */
+      if (TREE_STATIC (var)
+      || DECL_EXTERNAL (var)
+      || (TREE_CODE (origvar) == SSA_NAME && use_register_for_decl (var)))
     align = MINIMUM_ALIGNMENT (TREE_TYPE (var),
                    TYPE_MODE (TREE_TYPE (var)),
                    TYPE_ALIGN (TREE_TYPE (var)));

That said, I really wonder if we shouldn't besides estimated stack alignment
track also what we really need, i.e. record stack alignment requirements
without any pessimistic assumptions, only bump it when we actually allocate
something on the stack that needs bigger alignment (when we create MEM
DECL_RTL, when say assign_stack_temp* creates stack slot that needs bigger
alignment, when RA spills something that needs bigger alignment etc.).  RA etc.
would work as is, but ix86_finalize_stack_realign_flags would look at the
actual value instead.

Consider say:
typedef double m256 __attribute__((vector_size (32)));
m256 bar (m256 x, m256 y);
m256 foo (m256 x, m256 y, m256 z)
{
  return bar (x + z, y - z) + (m256) { 1.0, 2.0, 3.0, 4.0 };
}

        vaddpd  %ymm2, %ymm0, %ymm0
        pushq   %rbp
        vsubpd  %ymm2, %ymm1, %ymm1
        movq    %rsp, %rbp
        andq    $-32, %rsp
        call    bar
        vaddpd  .LC0(%rip), %ymm0, %ymm0
        leave
        ret

pushq %rbp; movq %rsp, %rbp; andq $-32, %rsp; leave all seem to be completely
unnecessary to me (well, some push/pop or rsp -=4/+=4 would be needed to
maintain 128-bit stack alignment), bar doesn't take any argument on the stack,
there is no V4DFmode spilling, etc.  For leaf functions
ix86_finalize_stack_realign_flags already manages to avoid that if the stack
pointer is never touched and frame pointer isn't needed.
I guess by adding another integer to x_rtl and tracking this carefully we could
get rid of the dynamic stack realignment here, still likely
frame_pointer_needed would be set.  Wonder if we couldn't optimize that away
(unless user requested frame pointer) too in some cases if frame pointer
register is unused or only used to look at arguments before stack is first
decremented.

[Bug middle-end/47735] [4.7/4.8/4.9 Regression] Unnecessary adjustments to stack pointer

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Jan  9 20:12:36 2014
New Revision: 206493

URL: http://gcc.gnu.org/viewcvs?rev=206493&root=gcc&view=rev
Log:
    PR middle-end/47735
    * cfgexpand.c (expand_one_var): For SSA_NAMEs, if the underlying
    var satisfies use_register_for_decl, just take into account type
    alignment, rather than decl alignment.

    * gcc.target/i386/pr47735.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr47735.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgexpand.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/47735] [4.7/4.8/4.9 Regression] Unnecessary adjustments to stack pointer

[law at redhat dot com]

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

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

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

--- Comment #6 from Jeffrey A. Law <law at redhat dot com> ---
Fixed by Jakub's commit on the trunk.  With that patch I get the same code as
GCC 4.4 for the included testcase.