[Bug target/55023] New: hppa: wrong code generated with tail call optimisation

[Bug target/55023] New: hppa: wrong code generated with tail call optimisation

[mans at mansr dot com]

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

             Bug #: 55023
           Summary: hppa: wrong code generated with tail call optimisation
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mans@mansr.com


The following code is compiled incorrectly with flags -O2 -fno-inline: the
third argument to the tail called function is not computed at all.  This
problem is present in all versions since 4.3.  4.1 and 4.2 compile it
correctly.

The error -fno-inline flag is needed to suppress inlining of the called
function; when inlined it is compiled correctly.  Providing only an external
declaration for foo() also gives correct results.

---8<---
typedef long long int64_t;

struct foo {
    int x;
    int y;
};

int64_t foo(int64_t a, int64_t b, int64_t c)
{
    return a + b + c;
}

int64_t bar(int64_t a, struct foo bq, struct foo cq)
{
    int64_t b = bq.x + bq.y;
    int64_t c = cq.x + cq.y;
    return foo(a, b, c);
}
--->8---

This is the generated code:

    .align 4
.globl bar
    .type    bar, @function
bar:
    .PROC
    .CALLINFO FRAME=0,NO_CALLS,SAVE_RP
    .ENTRY
    stw %r23,-48(%r30)
    stw %r24,-44(%r30)
    ldw -44(%r30),%r28
    stw %r2,-20(%r30)
    ldw -48(%r30),%r24
    addl %r24,%r28,%r24
    extrs %r24,0,1,%r23
    ldw -20(%r30),%r2
    bl foo,%r0
    nop
    nop
    .EXIT
    .PROCEND

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

--- Comment #1 from John David Anglin <danglin at gcc dot gnu.org> ---
Created attachment 33907
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33907&action=edit
fwprop2 dump

Looks like things go bad in dse1 pass.

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

--- Comment #2 from John David Anglin <danglin at gcc dot gnu.org> ---
Created attachment 33908
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33908&action=edit
dse1 dump

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33907|0                           |1
        is obsolete|                            |

--- Comment #3 from John David Anglin <danglin at gcc dot gnu.org> ---
Created attachment 33909
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33909&action=edit
cse2 dump

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[law at redhat dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-11-07
     Ever confirmed|0                           |1

--- Comment #4 from Jeffrey A. Law <law at redhat dot com> ---
Agreed, seems that RTL DSE is eliminating the stores.   Presumably its not
considering the call as potentially reading the argument stores.

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[dave.anglin at bell dot net]

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

--- Comment #5 from dave.anglin at bell dot net ---
On 11/7/2014 5:13 PM, law at redhat dot com wrote:
> Agreed, seems that RTL DSE is eliminating the stores.   Presumably its not
> considering the call as potentially reading the argument stores.
That's what I thought. When sibcalls are disabled, the normal call has
the same set of uses as the sibcall, but the stores are not eliminated.

Dave

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[law at redhat dot com]

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

--- Comment #6 from Jeffrey A. Law <law at redhat dot com> ---
Is the setup for the memory store different?   ie, in the sibcall case are we
making it hard for DSE to see that we have argument stores?  different base
register perhaps?

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[dave.anglin at bell dot net]

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

--- Comment #7 from dave.anglin at bell dot net ---
On 7-Nov-14, at 5:24 PM, law at redhat dot com wrote:

> Is the setup for the memory store different?   ie, in the sibcall  
> case are we
> making it hard for DSE to see that we have argument stores?   
> different base
> register perhaps?

Your suspicion appears correct.  When we optimize sibling calls, the  
argument
stores use the frame pointer (%r3).  When sibcalls are not optimized,  
the stores
use the stack pointer (%r30).

Dave
--
John David Anglin    dave.anglin@bell.net

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[dave.anglin at bell dot net]

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

--- Comment #9 from dave.anglin at bell dot net ---
After some more digging, I think the problem is in dse, not dce.  It  
deletes this instruction
which stores part of the sibcall arguments:

(insn 31 27 50 2 (set (mem:SI (reg/f:SI 115) [0  S4 A64])
         (reg:SI 125)) pr55023.c:17 40 {*pa.md:2204}
      (expr_list:REG_DEAD (reg:SI 125)
         (expr_list:REG_DEAD (reg/f:SI 115)
             (nil))))

**scanning insn=31
   mem: (reg/f:SI 115)

    after canon_rtx address: (plus:SI (reg/f:SI 3 %r3)
     (const_int -56 [0xffffffffffffffc8]))
   gid=0 offset=-56
  processing const base store gid=0[-56..-52)
     trying store in insn=26 gid=0[-52..-48)
mems_found = 1, cannot_delete = false
[...]
Locally deleting insn 31
deferring deletion of insn with uid = 31.
Locally deleting insn 26
deferring deletion of insn with uid = 26.

The argument setup for this argument is:

(insn 25 22 26 2 (set (reg/f:SI 124)
         (plus:SI (reg/f:SI 3 %r3)
             (const_int -52 [0xffffffffffffffcc]))) pr55023.c:17 112  
{addsi3}
      (nil))
(insn 26 25 27 2 (set (mem:SI (reg/f:SI 124) [0  S4 A32])         
(reg:SI 119 [ D.1503 ])) pr55023.c:17 40 {*pa.md:2204}
      (expr_list:REG_DEAD (reg/f:SI 124)
         (nil)))
(insn 27 26 31 2 (set (reg:SI 125)
         (ashiftrt:SI (reg:SI 119 [ D.1503 ])
             (const_int 31 [0x1f]))) pr55023.c:17 174 {*pa.md:6548}
      (expr_list:REG_DEAD (reg:SI 119 [ D.1503 ])
         (nil)))
(insn 31 27 50 2 (set (mem:SI (reg/f:SI 115) [0  S4 A64])
         (reg:SI 125)) pr55023.c:17 40 {*pa.md:2204}      
(expr_list:REG_DEAD (reg:SI 125)
         (expr_list:REG_DEAD (reg/f:SI 115)
             (nil))))

Pseudo "SI 115" was loaded earlier:

(insn 16 12 20 2 (set (reg/f:SI 115)
         (plus:SI (reg/f:SI 3 %r3)
             (const_int -56 [0xffffffffffffffc8]))) pr55023.c:16 112  
{addsi3}
      (nil))

Dave
--
John David Anglin    dave.anglin@bell.net

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[law at redhat dot com]

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

--- Comment #10 from Jeffrey A. Law <law at redhat dot com> ---
IIRC, the PA was a bit unique in how we had to set up the virtual registers due
to the combination of stack growth and arg growth directions.  It may be the
case that after virtual register instantiation we're left with something
different than is seen on any other architecture.  Thus the potential oversight
in DSE's handling of argument setup, particularly with sibcalls which use the
parent's stack.

But that's just speculation based on knowledge from 2 decades ago...  Take it
with a grain of salt...

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |UNCONFIRMED
                 CC|                            |rsandifo at gcc dot gnu.org
     Ever confirmed|1                           |0

--- Comment #11 from John David Anglin <danglin at gcc dot gnu.org> ---
The sibcall arguments are relative to the frame/argument pointer.  They
are the same on the 32-bit target and often are eliminated in favour of
the stack pointer.

On the 64-bit target, sibcalls are disabled because there is no relation
between the argument pointer and stack pointer.

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

John David Anglin <danglin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|2014-11-07 00:00:00         |2014-11-22
     Ever confirmed|0                           |1

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

--- Comment #12 from John David Anglin <danglin at gcc dot gnu.org> ---
Breakpoint 3, delete_dead_store_insn (insn_info=0x1b5ccf0)
    at ../../gcc/gcc/dse.c:948
948      if (!dbg_cnt (dse))
(gdb) p *insn_info
$3 = {cannot_delete = false, wild_read = false, non_frame_wild_read = false, 
  frame_read = false, stack_pointer_based = false, 
  contains_cselib_groups = false, insn = 0xfad93f28, store_rec = 0x1b60288, 
  read_rec = 0x0, fixed_regs_live = 0x1b52234, prev_insn = 0x1b5ccd0, 
  next_local_store = 0x1b5ccb0}
(gdb) p debug_rtx (insn_info->insn)
(insn 31 27 50 2 (set (mem:SI (reg/f:SI 115) [0  S4 A64])
        (reg:SI 125)) pr55023.c:17 40 {*pa.md:2204}
     (expr_list:REG_DEAD (reg:SI 125)
        (expr_list:REG_DEAD (reg/f:SI 115)
            (nil))))
$4 = void

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

--- Comment #13 from John David Anglin <danglin at gcc dot gnu.org> ---
Created attachment 34086
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34086&action=edit
Patch

Testing...

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[danglin at gcc dot gnu.org]

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

--- Comment #14 from John David Anglin <danglin at gcc dot gnu.org> ---
Author: danglin
Date: Mon Dec 22 23:10:18 2014
New Revision: 219037

URL: https://gcc.gnu.org/viewcvs?rev=219037&root=gcc&view=rev
Log:
    PR target/55023
    * dse.c (scan_insn): Treat sibling call as though it does a wild read.
    * testsuite/gcc.dg/pr55023.c: New file.


Added:
    trunk/gcc/testsuite/gcc.dg/pr55023.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c
    trunk/gcc/testsuite/ChangeLog

[Bug target/55023] hppa: wrong code generated with tail call optimisation

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Jan  8 21:29:44 2015
New Revision: 219361

URL: https://gcc.gnu.org/viewcvs?rev=219361&root=gcc&view=rev
Log:
    PR target/55023
    PR middle-end/64388
    * dse.c (struct insn_info): Mention frame_read set also
    before reload for tail calls on some targets.
    (scan_insn): Revert 2014-12-22 change.  Set frame_read
    also before reload for tail calls if
    HARD_FRAME_POINTER_IS_ARG_POINTER.  Call add_wild_read
    instead of add_non_frame_wild_read for non-const/memset
    tail calls after reload.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dse.c