Bug 21964 - [3.4 Regression] broken tail call at -O2 or more
Summary: [3.4 Regression] broken tail call at -O2 or more
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 3.4.4
: P2 normal
Target Milestone: 3.4.5
Assignee: Richard Sandiford
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: patch, wrong-code
Depends on:
Blocks:
 
Reported: 2005-06-08 17:11 UTC by Benjamin Herr
Modified: 2005-08-08 07:50 UTC (History)
2 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build: x86_64-pc-linux-gnu
Known to work: 4.0.0 4.1.0 2.95.3
Known to fail: 3.0.4 3.4.0
Last reconfirmed: 2005-08-01 08:55:27


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Herr 2005-06-08 17:11:11 UTC
Optimisations seem to cause the post increment in the recursive function call to
foo() to pass the increased value to the function.

Expected output, and output when compiled without -O2:
0
0
0
0
...

Actual output with -O2 or -O3 or...:
0
1
2
3
...

#include <stdio.h>
 
void foo(int n);
 
int main(void) {
    foo(0);
 
    return 0;
}
 
void foo(int n) {
    printf("%d\n", n);
    foo(n++);
}
Comment 1 Andrew Pinski 2005-06-08 18:13:50 UTC
Confirmed, only a regression in 3.4.x.
Comment 2 Steven Bosscher 2005-06-09 09:20:04 UTC
Here is the initial RTL for the call to foo: 
 
(call_insn 44 20 45 (call_placeholder 40 31 22 27 (cond [ 
  (const_string "normal") (sequence [ 
    (insn 40 0 39 (set (reg:SI 63) 
                (reg/v:SI 58 [ n ])) -1 (nil) 
            (nil)) 
    (insn 39 40 41 (parallel [ 
                    (set (reg/v:SI 58 [ n ]) 
                        (plus:SI (reg/v:SI 58 [ n ]) 
                            (const_int 1 [0x1]))) 
                    (clobber (reg:CC 17 flags)) 
                ]) -1 (nil) 
            (nil)) 
    (insn 41 39 42 (set (reg:SI 62) 
                (reg:SI 63)) -1 (nil) 
            (nil)) 
    (insn 42 41 43 (set (reg:SI 5 di) 
                (reg:SI 62)) -1 (nil) 
            (nil)) 
    (call_insn 43 42 0 (call (mem:QI (symbol_ref:DI ("foo") [flags 0x3] 
<function_decl 0x2a9592b9c0 foo>) [0 S1 A8]) 
                (const_int 0 [0x0])) -1 (nil) 
            (nil) 
            (expr_list (use (reg:SI 5 di)) 
                (nil))) 
    ]) 
  (const_string "tail_call") (sequence [ 
    (note 31 0 32 NOTE_INSN_DELETED) 
    (note 32 31 34 NOTE_INSN_DELETED) 
    (insn 34 32 33 (set (reg:SI 61) 
                (reg/v:SI 58 [ n ])) -1 (nil) 
            (nil)) 
    (insn 33 34 35 (parallel [ 
                    (set (reg/v:SI 58 [ n ]) 
                        (plus:SI (reg/v:SI 58 [ n ]) 
                            (const_int 1 [0x1]))) 
                    (clobber (reg:CC 17 flags)) 
                ]) -1 (nil) 
            (nil)) 
    (insn 35 33 36 (set (reg:SI 60) 
                (reg:SI 61)) -1 (nil) 
            (nil)) 
    (insn 36 35 37 (set (reg:SI 5 di) 
                (reg:SI 60)) -1 (nil) 
            (nil)) 
    (call_insn/j 37 36 38 (call (mem:QI (symbol_ref:DI ("foo") [flags 0x3] 
<function_decl 0x2a9592b9c0 foo>) [0 S1 A8]) 
                (const_int 0 [0x0])) -1 (nil) 
            (nil) 
            (expr_list (use (reg:SI 5 di)) 
                (nil))) 
    (barrier 38 37 0) 
    ]) 
  (const_string "tail_recursion") (sequence [ 
    (note 22 0 23 NOTE_INSN_DELETED) 
    (note 23 22 25 NOTE_INSN_DELETED) 
    (insn 25 23 26 (set (reg:SI 59) 
                (reg/v:SI 58 [ n ])) -1 (nil) 
            (nil)) 
    (insn 26 25 24 (set (reg/v:SI 58 [ n ]) 
                (reg:SI 59)) -1 (nil) 
            (nil)) 
    (insn 24 26 28 (parallel [ 
                    (set (reg/v:SI 58 [ n ]) 
                        (plus:SI (reg/v:SI 58 [ n ]) 
                            (const_int 1 [0x1]))) 
                    (clobber (reg:CC 17 flags)) 
                ]) -1 (nil) 
            (nil)) 
    (jump_insn 28 24 29 (set (pc) 
                (label_ref 27)) -1 (nil) 
            (nil)) 
    (barrier 29 28 30) 
    (barrier 30 29 0) 
    ]) 
  ])) -1 (nil) 
    (nil) 
    (nil)) 
 
 
You have to set debug_call_placeholder_verbose = 1 in print-rtl.c to 
see what is going on. 
Comment 3 Steven Bosscher 2005-06-09 09:22:14 UTC
In this case we obviously take the "tail_recursion" sequence, which is 
already wrong: 
 
  (const_string "tail_recursion") (sequence [ 
    (note 22 0 23 NOTE_INSN_DELETED) 
    (note 23 22 25 NOTE_INSN_DELETED) 
    (insn 25 23 26 (set (reg:SI 59) 
                (reg/v:SI 58 [ n ])) -1 (nil) 
            (nil)) 
    (insn 26 25 24 (set (reg/v:SI 58 [ n ]) 
                (reg:SI 59)) -1 (nil) 
            (nil)) 
    (insn 24 26 28 (parallel [ 
                    (set (reg/v:SI 58 [ n ]) 
                        (plus:SI (reg/v:SI 58 [ n ]) 
                            (const_int 1 [0x1]))) 
                    (clobber (reg:CC 17 flags)) 
                ]) -1 (nil) 
            (nil)) 
    (jump_insn 28 24 29 (set (pc) 
                (label_ref 27)) -1 (nil) 
            (nil)) 
    (barrier 29 28 30) 
    (barrier 30 29 0) 
    ]) 
 
Note that the "n++" (insn 24) is before the jump to the tail recursion 
label at the head of the function (jump_insn 28). 
 
So the initial RTL generation is wrong. 
 
 
Comment 4 Giovanni Bajo 2005-06-09 10:33:18 UTC
can you binsearch this on the 3.4 branch?
Comment 5 Steven Bosscher 2005-06-09 11:25:13 UTC
Why would you want to binsearch this?  GCC 3.0.4 was already broken,
according to the "Known to fail" list, while 2.95.3 is "Known to work".
And because the sibcall pass was new in GCC 3.0, the odds are that
this was broken from the beginning.
Comment 6 Giovanni Bajo 2005-06-09 11:52:44 UTC
Ah sorry, for some reason I misread the bug and believed it worked in 3.3.
Comment 7 Richard Sandiford 2005-08-01 08:55:27 UTC
Testing a patch
Comment 8 GCC Commits 2005-08-08 07:45:41 UTC
Subject: Bug 21964

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rsandifo@gcc.gnu.org	2005-08-08 07:45:22

Modified files:
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: pr21964-1.c 

Log message:
	PR middle-end/21964
	* gcc.c-torture/execute/pr21964-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5891&r2=1.5892
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/pr21964-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 9 GCC Commits 2005-08-08 07:46:24 UTC
Subject: Bug 21964

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	rsandifo@gcc.gnu.org	2005-08-08 07:46:18

Modified files:
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.c-torture/execute: pr21964-1.c 

Log message:
	PR middle-end/21964
	* gcc.c-torture/execute/pr21964-1.c: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.321&r2=1.5084.2.322
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/pr21964-1.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1

Comment 11 Richard Sandiford 2005-08-08 07:50:16 UTC
Patch applied to 3.4.  Testcase also applied to mainline and 4.0.