Bug 42313 - FAIL: gcc.target/i386/builtin-unreachable.c scan-assembler-not %e[bs]p on i686 darwin
Summary: FAIL: gcc.target/i386/builtin-unreachable.c scan-assembler-not %e[bs]p on i68...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2009-12-06 20:10 UTC by Jack Howarth
Modified: 2010-09-08 01:05 UTC (History)
4 users (show)

See Also:
Host:
Target: i?86-apple-darwin*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
preprocessed source forgcc.target/i386/builtin-unreachable.c on i686-apple-darwin10 (182 bytes, text/plain)
2009-12-06 20:12 UTC, Jack Howarth
Details
assembly file for gcc.target/i386/builtin-unreachable.c on i686-apple-darwin10 (100 bytes, text/plain)
2009-12-06 20:13 UTC, Jack Howarth
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jack Howarth 2009-12-06 20:10:54 UTC
On i686-apple-darwin we are failing the testcase...

FAIL: gcc.target/i386/builtin-unreachable.c scan-assembler-not %e[bs]p

gcc-4 -v
Using built-in specs.
COLLECT_GCC=gcc-4
COLLECT_LTO_WRAPPER=/sw/lib/gcc4.5/libexec/gcc/i686-apple-darwin10/4.5.0/lto-wrapper
Target: i686-apple-darwin10
Configured with: ../gcc-4.5-20091205/configure --prefix=/sw --prefix=/sw/lib/gcc4.5 --mandir=/sw/share/man --infodir=/sw/share/info --enable-languages=c,c++,fortran,objc,obj-c++,java --with-gmp=/sw --with-libiconv-prefix=/sw --with-ppl=/sw --with-cloog=/sw --with-mpc=/sw --with-system-zlib --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib --disable-libjava-multilib --with-arch=nocona --with-tune=generic --build=i686-apple-darwin10 --host=i686-apple-darwin10 --target=i686-apple-darwin10
Thread model: posix
gcc version 4.5.0 20091205 (experimental) (GCC)
Comment 1 Jack Howarth 2009-12-06 20:12:20 UTC
Created attachment 19242 [details]
preprocessed source forgcc.target/i386/builtin-unreachable.c on i686-apple-darwin10
Comment 2 Jack Howarth 2009-12-06 20:13:13 UTC
Created attachment 19243 [details]
assembly file for gcc.target/i386/builtin-unreachable.c on i686-apple-darwin10

Generated with...

/sw/src/fink.build/gcc45-4.4.999-20091205/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc45-4.4.999-20091205/darwin_objdir/gcc/ /sw/src/fink.build/gcc45-4.4.999-20091205/gcc-4.5-20091205/gcc/testsuite/gcc.target/i386/builtin-unreachable.c -O2 -fomit-frame-pointer --save-temps -S -o builtin-unreachable.s
Comment 3 David Daney 2009-12-07 17:27:11 UTC
It is failing because the redundant stack pointer adjustments are not being removed.

This test is passing for me on  i686-pc-linux-gnu at r154987, so I think it must be darwin specific.

If for some reason darwin requires a stack frame, perhaps the:

/* { dg-final { scan-assembler-not "%e\[bs\]p" } } */

part could be made conditional on  darwin.
Comment 4 Jack Howarth 2009-12-08 14:13:27 UTC
Mike Stump says that the frame can be optimized away on darwin.  However, Apple's 4.2.1 compiler in darwin10 also appears to leave the stack frame...

 [MacPro:~/bug] howarth% gcc-4.2 -O2 -fomit-frame-pointer -m32 --save-temps -c builtin-unreachable.c
 [MacPro:~/bug] howarth% more builtin-unreachable.s
        .text
        .align 4,0x90
 .globl _h
 _h:
        subl    $12, %esp
        movl    16(%esp), %eax
 	 cmpb    $0, (%eax)
        je      L2
        call    ___builtin_unreachable
 L2:
        movl    $1, %eax
        addl    $12, %esp
        ret
        .subsections_via_symbols      
            
Comment 5 David Daney 2009-12-08 17:34:44 UTC
(In reply to comment #4)
> Mike Stump says that the frame can be optimized away on darwin.  However,
> Apple's 4.2.1 compiler in darwin10 also appears to leave the stack frame...

That compiler doesn't implement __builtin_unreachable(), so is irrelevant to the discussion of this bug.

Of interest is why, in a leaf function that doesn't use the stack, the stack pointer is adjusted.

A darwin hacker will probably have to look into it as I only have access to GNU/Linux targets.


Comment 6 Andrew Pinski 2009-12-08 19:51:19 UTC
I think this comes down to an alignment issue. On darwin, the stack has to be aligned to 16bytes so something inside i386.c is deciding that we to allocate the stack frame as there was something on the stack and we have to align it again.
Comment 7 Mike Stump 2010-03-03 16:56:10 UTC
I fixed the test case to not expect the unimplemented optimization in r157197, however, it would be nice to ensure the async signal handlers can handle unaligned stacks and to perform this optimization.  I'm fairly certain the signal handers have to cope as code gen can do:

_h:
        subl    $12, %esp
        movl    $1, %eax
        addl    $12, %esp
        ret

for int h () { return 1; } and certainly we can take a signal at _h+0, where esp isn't aligned.  Given that, we can omit sp alignments for leaf functions (and no tail calls either), even on machines where otherwise an alignment is required, as long as any variables on the stack are correctly aligned.
 
When this optimization is added, we can undo the r157197 change. 
Comment 8 Jack Howarth 2010-09-04 19:13:04 UTC
(In reply to comment #7)
> I fixed the test case to not expect the unimplemented optimization in r157197,
> however, it would be nice to ensure the async signal handlers can handle
> unaligned stacks and to perform this optimization.  I'm fairly certain the
> signal handers have to cope as code gen can do:
> 
> _h:
>         subl    $12, %esp
>         movl    $1, %eax
>         addl    $12, %esp
>         ret
> 
> for int h () { return 1; } and certainly we can take a signal at _h+0, where
> esp isn't aligned.  Given that, we can omit sp alignments for leaf functions
> (and no tail calls either), even on machines where otherwise an alignment is
> required, as long as any variables on the stack are correctly aligned.
> 
> When this optimization is added, we can undo the r157197 change. 
> 

My proposed patch to fix PR36502...

http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00237.html

that enables stack realignment on intel darwin also solves this PR as well. Comparing the output from gcc trunk before and after my patch, I see...

--- builtin-unreachable.s	2010-09-04 15:12:40.000000000 -0400
+++ builtin-unreachable.trunk_patched	2010-09-04 15:02:45.000000000 -0400
@@ -3,11 +3,7 @@
 	.globl _h
 _h:
 LFB0:
-	subl	$12, %esp
-LCFI0:
 	movl	$1, %eax
-	addl	$12, %esp
-LCFI1:
 	ret
 LFE0:
 	.section __TEXT,__eh_frame,coalesced,no_toc+strip_static_syms+live_support
@@ -39,16 +35,6 @@
 	.set L$set$2,LFE0-LFB0
 	.long L$set$2
 	.byte	0
-	.byte	0x4
-	.set L$set$3,LCFI0-LFB0
-	.long L$set$3
-	.byte	0xe
-	.byte	0x10
-	.byte	0x4
-	.set L$set$4,LCFI1-LCFI0
-	.long L$set$4
-	.byte	0xe
-	.byte	0x4
 	.align 2
 LEFDE1:
 	.subsections_via_symbols

Comment 9 Jack Howarth 2010-09-04 21:20:28 UTC
Updated patch to reflect the wider coverage of PRs fixed...
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00365.html
Comment 10 hjl@gcc.gnu.org 2010-09-07 21:19:19 UTC
Subject: Bug 42313

Author: hjl
Date: Tue Sep  7 21:18:55 2010
New Revision: 163971

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163971
Log:
Redefine STACK_BOUNDARY/PREFERRED_STACK_BOUNDARY for Darwin/x86.

gcc/

2010-09-07  H.J. Lu  <hjl.tools@gmail.com>
	    Jack Howarth <howarth@bromo.med.uc.edu>

	PR target/36502
	PR target/42313
	PR target/44651
	* gcc/config/i386/darwin.h (STACK_BOUNDARY): Redefine as 128 for
	profiling or 64-bit MS_ABI and as BITS_PER_WORD otherwise.
	(PREFERRED_STACK_BOUNDARY): Replace STACK_BOUNDARY with 128 in
	MAX macro.

gcc/testsuite/

2010-09-07  Jack Howarth <howarth@bromo.med.uc.edu>

	PR target/36502
	* gcc.target/i386/pr36502.c: New test.

	PR target/42313
	PR target/44651
	* gcc.target/i386/builtin-unreachable.c: Don't skip on darwin.
	* gcc/testsuite/gcc.dg/stack-usage-1.c: Use default on i386/Darwin.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr36502.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/darwin.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/stack-usage-1.c
    trunk/gcc/testsuite/gcc.target/i386/builtin-unreachable.c

Comment 11 Jack Howarth 2010-09-08 01:05:30 UTC
Fixed on trunk at r163971.
Comment 12 hjl@gcc.gnu.org 2010-09-10 16:13:10 UTC
Subject: Bug 42313

Author: hjl
Date: Fri Sep 10 16:12:46 2010
New Revision: 164197

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164197
Log:
Redefine STACK_BOUNDARY/PREFERRED_STACK_BOUNDARY for Darwin/x86.

gcc/

2010-09-10  Jack Howarth <howarth@bromo.med.uc.edu>

	Backport from mainline
	2010-09-07  H.J. Lu  <hjl.tools@gmail.com>
		    Jack Howarth <howarth@bromo.med.uc.edu>

	PR target/36502
	PR target/42313
	PR target/44651
	* config/i386/darwin.h (STACK_BOUNDARY): Redefine as 128 for
	profiling or 64-bit MS_ABI and as BITS_PER_WORD otherwise.
	(PREFERRED_STACK_BOUNDARY): Replace STACK_BOUNDARY with 128 in
	MAX macro.

gcc/testsuite/

2010-09-10  Jack Howarth <howarth@bromo.med.uc.edu>

	Backport from mainline
	2010-09-07  Jack Howarth <howarth@bromo.med.uc.edu>

	PR target/36502
	* gcc.target/i386/pr36502.c: New test.

	PR target/42313
	PR target/44651
	* gcc.target/i386/builtin-unreachable.c: Don't skip on darwin.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.target/i386/pr36502.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/i386/darwin.h
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/gcc.target/i386/builtin-unreachable.c