[Bug target/45701] New: [regression 4.5] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[Bug target/45701] New: [regression 4.5] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[qiyao at gcc dot gnu dot org]

This was implemented in gcc-4.5, still works in gcc-4.5 branch.
2009-06-02 Richard Earnshaw <rearnsha@arm.com>

        * arm.c (arm_get_frame_offsets): Prefer using r3 for padding a
        push/pop multiple to 8-byte alignment.
However, it doesn't work any longer on gcc-4.6 trunk.  Reproduced on the
following steps,

$ arm-none-linux-gnueabi-gcc -Os -mthumb -march=armv7-a bashline.c -S

On gcc-4.5 branch, r3 is used to keep stack alignment, which is expected.
history_expand_line_internal:
        @ args = 0, pretend = 0, frame = 0 
        @ frame_needed = 0, uses_anonymous_args = 0 
        push    {r3, r4, r5, r6, r7, lr} 

However, on gcc-4.6 trunk, we can see that, r8 is used to keep stack alignment,
which is *not* expected.
history_expand_line_internal:
        @ args = 0, pretend = 0, frame = 0 
        @ frame_needed = 0, uses_anonymous_args = 0 
        push    {r4, r5, r6, r7, r8, lr}


-- 
           Summary: [regression 4.5] Fail to prefer using r3 for padding a
                    push/pop multiple to 8-byte alignment
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: qiyao at gcc dot gnu dot org
  GCC host triplet: i486-build_pc-linux-gnu
GCC target triplet: arm-unknown-linux-gnueabi


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

[Bug target/45701] [regression 4.5] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[qiyao at gcc dot gnu dot org]

------- Comment #1 from qiyao at gcc dot gnu dot org  2010-09-17 10:52 -------
Created an attachment (id=21816)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21816&action=view)
Test case


-- 


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

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[regression 4.5] Fail to    |[4.6 Regression] Fail to
                   |prefer using r3 for padding |prefer using r3 for padding
                   |a push/pop multiple to 8-   |a push/pop multiple to 8-
                   |byte alignment              |byte alignment
   Target Milestone|---                         |4.6.0


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

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[qiyao at gcc dot gnu dot org]

------- Comment #2 from qiyao at gcc dot gnu dot org  2010-09-19 13:19 -------
In arm.c:arm_get_frame_offsets (void)

          if (!crtl->tail_call_emit
              && arm_size_return_regs () <= 12
              && (offsets->saved_regs_mask & (1 << 3)) == 0)
            {
              reg = 3;
            }

In gcc-4.5, crtl->tail_call_emit is always false, so the condition is true. 
However, in gcc-4.6, crtl->tail_call_emit can be true.  My stupid question is
'why can't we use r3 when we do tail call optimization?'.  I read one sentence
in comment, 'We try to avoid using the arg registers (r0 -r3) as they might be 
used to pass values in a tail call.'.  Is it the answer to my question?

If that is the answer, in oder to fix this bug, we may refine the condition to
'tail call insns are emitted, and r3 is used to pass values to tail call', like
this,

  if (!(crtl->tail_call_emit && used_to_pass_values (3))
       && ...
       && ...)

Is this a correct fix?


-- 


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

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[qiyao at gcc dot gnu dot org]

------- Comment #3 from qiyao at gcc dot gnu dot org  2010-09-22 02:18 -------
Patch is submitted to gcc-patches for review
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01741.html


-- 

qiyao at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |qiyao at gcc dot gnu dot org


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

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[dnovillo at gcc dot gnu.org]

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

--- Comment #12 from Diego Novillo <dnovillo at gcc dot gnu.org> 2011-02-02 17:46:52 UTC ---
Author: dnovillo
Date: Wed Feb  2 17:46:50 2011
New Revision: 169583

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169583
Log:
    PR target/45701
    * config/arm/arm.c (any_sibcall_uses_r3): New function.
    (arm_get_frame_offsets): Use it.

2011-01-25  Yao Qi  <yao@codesourcery.com>

        PR target/45701
        * gcc.target/arm/pr45701-1.c: New test.
        * gcc.target/arm/pr45701-2.c: New test.
    * gcc.target/arm/pr45701-3.c: New test.

Added:
    branches/google/integration/gcc/testsuite/gcc.target/arm/pr45701-1.c
    branches/google/integration/gcc/testsuite/gcc.target/arm/pr45701-2.c
    branches/google/integration/gcc/testsuite/gcc.target/arm/pr45701-3.c
Modified:
    branches/google/integration/gcc/ChangeLog
    branches/google/integration/gcc/config/arm/arm.c
    branches/google/integration/gcc/testsuite/ChangeLog

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-25 16:22:45 UTC ---
Author: jakub
Date: Tue Jan 25 16:22:34 2011
New Revision: 169240

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169240
Log:
    PR target/45701
    * config/arm/arm.c (any_sibcall_uses_r3): New function.
    (arm_get_frame_offsets): Use it.

2011-01-25  Yao Qi  <yao@codesourcery.com>

        PR target/45701
        * gcc.target/arm/pr45701-1.c: New test.
        * gcc.target/arm/pr45701-2.c: New test.
    * gcc.target/arm/pr45701-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/arm/pr45701-1.c
    trunk/gcc/testsuite/gcc.target/arm/pr45701-2.c
    trunk/gcc/testsuite/gcc.target/arm/pr45701-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-25 16:24:30 UTC ---
Fixed.

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[ramana at gcc dot gnu.org]

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

--- Comment #9 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2011-01-19 10:39:28 UTC ---
(In reply to comment #7)
> On IRC you've mentioned some TLS test issues, were they caused by this patch or
> unrelated?

Sorry about the delay in responding. With trunk I see no new regressions with
my testing configurations . The tls failures were unrelated to this patch. 

Ok to submit 



Ramana

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[ramana.r at gmail dot com]

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

--- Comment #8 from Ramana Radhakrishnan <ramana.r at gmail dot com> 2011-01-15 15:16:59 UTC ---
On Sat, Jan 15, 2011 at 1:36 PM, jakub at gcc dot gnu.org
<gcc-bugzilla@gcc.gnu.org> wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45701
>
> --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-15 13:35:46 UTC ---
> On IRC you've mentioned some TLS test issues, were they caused by this patch or
> unrelated?

Been travelling this week. Should be able to get answers once I can
access my board next week.


cheers
Ramana

>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug.
>

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-15 13:35:46 UTC ---
On IRC you've mentioned some TLS test issues, were they caused by this patch or
unrelated?

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-04 10:37:36 UTC ---
(In reply to comment #5)
> I'll test this.

Have you managed to test it already?  Thanks.

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[ramana at gcc dot gnu.org]

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

--- Comment #5 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2010-12-24 11:04:08 UTC ---
Ok I'll test this. (In reply to comment #4)
> Created attachment 22835 [details]
> gcc46-pr45701.patch
> 
> Remembering pointers to tail call insns is both wasteful (allocates stuff that
> is never used anywhere but on arm), error-prone (nothing adjusts the list if
> tail calls are deleted, or duplicated, etc.) and unnecessary, because
> the list is trivially available from the IL.
> 
> Here is completely untested patch (tested just on the 3 testcases with a
> cross).
> Could anyone interested in ARM target please bootstrap/regtest this?
> We have way too many open P1 bugs.

I'll test this.

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-12-21 12:30:25 UTC ---
Created attachment 22835
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22835
gcc46-pr45701.patch

Remembering pointers to tail call insns is both wasteful (allocates stuff that
is never used anywhere but on arm), error-prone (nothing adjusts the list if
tail calls are deleted, or duplicated, etc.) and unnecessary, because
the list is trivially available from the IL.

Here is completely untested patch (tested just on the 3 testcases with a
cross).
Could anyone interested in ARM target please bootstrap/regtest this?
We have way too many open P1 bugs.

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[ramana at gcc dot gnu.org]

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

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.12.14 09:34:41
                 CC|                            |ramana at gcc dot gnu.org
     Ever Confirmed|0                           |1

[Bug target/45701] [4.6 Regression] Fail to prefer using r3 for padding a push/pop multiple to 8-byte alignment

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1