Between revisions 176012 and 176060 running gcc.dg/sibcall-6.c compiled with -O2 -foptimize-sibling-calls -fno-ipa-cp on x86_64-apple-darwin10 aborts.
This is due to revision 176042. I'll attach the working and failing assembly files.
Created attachment 25008 [details] working assembly
Created attachment 25009 [details] failing assembly
It also failed with -fPIC on Linux/x86-64. It should be disabled if PIC is enabled by default.
> It also failed with -fPIC on Linux/x86-64. It should be disabled > if PIC is enabled by default. Well, the test succeeds at revision 176041 and I don't see why it should be disabled after revision 176042.
It is triggered by http://gcc.gnu.org/git/?p=gcc.git;a=commit;h=cc0595c0a5a0ab03fba105bb1dbe856557b1a988
Kirill, this is a regression: [hjl@gnu-6 gcc]$ cat /tmp/x.i int (*ptr) (int); int foo (int f) { return (*ptr)(f); } [hjl@gnu-6 gcc]$ ./xgcc -B./ -O2 -foptimize-sibling-calls -fno-ipa-cp -S /tmp/x.i -fPIC [hjl@gnu-6 gcc]$ cat x.s .file "x.i" .text .p2align 4,,15 .globl foo .type foo, @function foo: .LFB0: .cfi_startproc subq $8, %rsp .cfi_def_cfa_offset 16 movq ptr@GOTPCREL(%rip), %rax call *(%rax) addq $8, %rsp .cfi_def_cfa_offset 8 ret PTR here isn't an argument.
Hi, I agree, this is a performance regression. Fix to tail-call optimization made it very conservative. By using some additional tweaks, we may relax it. However, my fix cured a stability problem (see, 49519 for details).
I see the same on s390x. gcc.dg/sibcall-6.c starts failing with r176042.
(In reply to comment #9) > I see the same on s390x. gcc.dg/sibcall-6.c starts failing with r176042. I also see this on Epiphany.
Created attachment 25858 [details] Patch to make mem_overlaps_already_clobbered_arg_p return false if no arguments were stored on stack This patch fixes the sibcall-6.c failure on Epiphany.
> This patch fixes the sibcall-6.c failure on Epiphany. On x86_64-apple-darwin10 too. Thanks.
This fixes the testcase on s390x. Tested with r181554. Thanks!
(In reply to comment #11) > Created attachment 25858 [details] > Patch to make mem_overlaps_already_clobbered_arg_p return false if no arguments > were stored on stack > > This patch fixes the sibcall-6.c failure on Epiphany. This patch is certainly correct, please submit it to gcc-patches. That said, this isn't enough. For i?86, it should be probably enough to revert Kyrill's patch and do something like: --- gcc/expr.c.jj 2011-11-21 16:22:02.000000000 +0100 +++ gcc/expr.c 2011-11-25 12:46:40.070831662 +0100 @@ -7452,7 +7452,8 @@ expand_expr_addr_expr_1 (tree exp, rtx t } if (modifier != EXPAND_INITIALIZER - && modifier != EXPAND_CONST_ADDRESS) + && modifier != EXPAND_CONST_ADDRESS + && modifier != EXPAND_SUM) result = force_operand (result, target); return result; } (untested so far, but if it works, is IMHO desirable in any case). Unfortunately we have ia64 which only allows registers in memory addresses and thus we'd miscompile e.g. struct S { long s[4]; }; __attribute__((noinline, noclone)) void bar (long r0, long r1, long r2, long r3, long r4, long r5, long r6, long r7, long r8, long r9, long r10, long r11) { asm volatile (""); } void foo (long r0, long r1, long r2, long r3, long r4, long r5, long r6, long r7, long x, struct S y) { bar (r0, r1, r2, r3, r4, r5, r6, r7, y.s[0], y.s[1], y.s[2], y.s[3]); } BTW, Kyrill's patch didn't deal with arbitrary pseudo plus constant or pseudo plus pseudo. Rejecting all pseudos is way too conservative though, tree-tailcall.c doesn't allow tailcalls if caller has address taken on any of the operands, thus the only case when pseudos point into the arguments area is when the pseudo is initialized in the current get_insns () list to something based on internal argument pointer. I'd say we could just look up (and cache) which pseudos in the current get_insns () list are crtl->args.internal_arg_pointer based and their corresponding offsets from it (if constant, or something that says they are variable).
Author: amylaar Date: Sat Nov 26 09:21:47 2011 New Revision: 181738 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181738 Log: PR middle-end/50074 * calls.c (mem_overlaps_already_clobbered_arg_p): Return false if no outgoing arguments have been stored so far. Modified: trunk/gcc/ChangeLog trunk/gcc/calls.c
See http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02413.html
Author: jakub Date: Tue Nov 29 08:48:41 2011 New Revision: 181800 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181800 Log: PR middle-end/50074 * expr.c (expand_expr_addr_expr_1): Don't call force_operand for EXPAND_SUM modifier. Modified: trunk/gcc/ChangeLog trunk/gcc/expr.c
Author: jakub Date: Mon Dec 5 08:15:23 2011 New Revision: 182000 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182000 Log: PR middle-end/51323 PR middle-end/50074 * calls.c (internal_arg_pointer_exp_state): New variable. (internal_arg_pointer_based_exp_1, internal_arg_pointer_exp_scan): New functions. (internal_arg_pointer_based_exp): New function. (mem_overlaps_already_clobbered_arg_p): Use it. (expand_call): Free internal_arg_pointer_exp_state.cache vector and clear internal_arg_pointer_exp_state.scan_start. * gcc.c-torture/execute/pr51323.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr51323.c Modified: trunk/gcc/ChangeLog trunk/gcc/calls.c trunk/gcc/testsuite/ChangeLog
Fixed.
Author: jakub Date: Thu Dec 8 13:36:40 2011 New Revision: 182112 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182112 Log: Backport from mainline 2011-12-05 Jakub Jelinek <jakub@redhat.com> Eric Botcazou <ebotcazou@adacore.com> PR middle-end/51323 PR middle-end/50074 * calls.c (internal_arg_pointer_exp_state): New variable. (internal_arg_pointer_based_exp_1, internal_arg_pointer_exp_scan): New functions. (internal_arg_pointer_based_exp): New function. (mem_overlaps_already_clobbered_arg_p): Use it. (expand_call): Free internal_arg_pointer_exp_state.cache vector and clear internal_arg_pointer_exp_state.scan_start. 2011-11-26 Joern Rennecke <joern.rennecke@embecosm.com> PR middle-end/50074 * calls.c (mem_overlaps_already_clobbered_arg_p): Return false if no outgoing arguments have been stored so far. 2011-12-05 Jakub Jelinek <jakub@redhat.com> Eric Botcazou <ebotcazou@adacore.com> PR middle-end/51323 PR middle-end/50074 * gcc.c-torture/execute/pr51323.c: New test. Added: branches/gcc-4_6-branch/gcc/testsuite/gcc.c-torture/execute/pr51323.c Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/calls.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog