Bug 36502 - i386/darwin generates unnecessary stack ops in every function
Summary: i386/darwin generates unnecessary stack ops in every function
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.4.0
: P3 normal
Target Milestone: 4.6.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2008-06-11 22:17 UTC by astrange+gcc@gmail.com
Modified: 2010-09-08 02:16 UTC (History)
5 users (show)

See Also:
Host:
Target: i386-apple-darwin*
Build:
Known to work:
Known to fail: 4.4.0 4.5.0
Last reconfirmed: 2010-01-02 20:52:27


Attachments
patch to properly use MIN_STACK_BOUNDARY on intel darwin. (731 bytes, patch)
2010-08-31 01:23 UTC, Jack Howarth
Details | Diff
rely on PREFERRED_STACK_BOUNDARY_DEFAULT for intel darwin (691 bytes, patch)
2010-08-31 05:13 UTC, Jack Howarth
Details | Diff
rely on PREFERRED_STACK_BOUNDARY_DEFAULT and MAIN_STACK_BOUNDARY (640 bytes, patch)
2010-08-31 15:39 UTC, Jack Howarth
Details | Diff
assembly from failing gcc.dg/nest.c execution test at -m32 on x86_64-apple-darwin10 (759 bytes, text/plain)
2010-08-31 16:36 UTC, Jack Howarth
Details
assembly from failing gcc.dg/torture/stackalign/alloca-4.c -O1 execution test at -m32 on x86_64-apple-darwin10 (1.42 KB, text/plain)
2010-08-31 16:42 UTC, Jack Howarth
Details
assembly from stock build of gcc.dg/nest.c execution test at -m32 on x86_64-apple-darwin10 (784 bytes, text/plain)
2010-08-31 19:50 UTC, Jack Howarth
Details
assembly from stock build of gcc.dg/torture/stackalign/alloca-4.c -O1 execution test at -m32 on x86_64-apple-darwin10 (1.50 KB, text/plain)
2010-08-31 19:55 UTC, Jack Howarth
Details
diff between nest.s.stock and nest.s show alignment changes in failing test case (574 bytes, text/plain)
2010-08-31 19:57 UTC, Jack Howarth
Details
diff between alloca-4.s.stock and alloca-4.s show alignment changes in failing test case (1.86 KB, text/plain)
2010-08-31 19:59 UTC, Jack Howarth
Details
Don't redefine STACK_BOUNDARY and replace STACK_BOUNDARY with 128. (736 bytes, patch)
2010-09-01 15:30 UTC, Jack Howarth
Details | Diff
A patrch (947 bytes, patch)
2010-09-01 19:49 UTC, H.J. Lu
Details | Diff
Another patch (484 bytes, patch)
2010-09-01 20:03 UTC, H.J. Lu
Details | Diff
don't redefine MAIN_STACK_BOUNDARY so -fstack-usage can work (1.25 KB, patch)
2010-09-03 01:58 UTC, Jack Howarth
Details | Diff
retain redefinition of MAIN_STACK_BOUNDARY as required (1.10 KB, patch)
2010-09-06 13:17 UTC, Jack Howarth
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description astrange+gcc@gmail.com 2008-06-11 22:17:44 UTC
> gcc -v
Using built-in specs.
Target: i386-apple-darwin9.2.2
Configured with: ../gcc/configure --prefix=/usr/local/gcc44 --enable-threads=posix --with-arch=core2 --with-tune=core2 --with-gmp=/sw --with-mpfr=/sw --disable-nls --disable-bootstrap --enable-checking=yes,rtl --enable-languages=c,c++,objc
Thread model: posix
gcc version 4.4.0 20080611 (experimental) (GCC) 

gcc changes esp in every function, even if it has no stack values. Given:
    int a;
    void f() {a++;}

> gcc -O -fomit-frame-pointer -fno-pic -S add.c
_f:
	subl	$12, %esp
	incl	_a
	addl	$12, %esp
	ret

Apple's GCC doesn't do this and neither does 4.4 on other systems (as far as I know).
Comment 1 Andrew Pinski 2008-06-11 23:43:06 UTC
This is due to alignment requirement interacting weirdly with the back-end.
Comment 2 Francois-Xavier Coudert 2010-01-02 20:52:27 UTC
Still present as of rev. 155544.
Comment 3 Richard Henderson 2010-08-30 18:20:30 UTC
It's caused by this:

config/i386/darwin.h:#define STACK_BOUNDARY 128

I think you want to delete that and allow STACK_BOUNDARY to be defined
by i386.h to UNITS_PER_WORD.  Non-leaf functions should be handled by

#define MIN_STACK_BOUNDARY 128

which should get you your ABI minimum for the callee.
Comment 4 Jack Howarth 2010-08-30 22:44:54 UTC
(In reply to comment #3)
Richard,
      The following patch fails to bootstrap...


Index: i386/darwin.h
===================================================================
--- i386/darwin.h       (revision 163661)
+++ i386/darwin.h       (working copy)
@@ -77,10 +77,12 @@
 
 /* On Darwin, the stack is 128-bit aligned at the point of every call.
    Failure to ensure this will lead to a crash in the system libraries
-   or dynamic loader.  */
-#undef STACK_BOUNDARY
-#define STACK_BOUNDARY 128
+   or dynamic loader. Let STACK_BOUNDARY be set using UNITS_PER_WORD as
+   non-leaf functions should be handled by MIN_STACK_BOUNDARY of 128. */
 
+#undef MIN_STACK_BOUNDARY
+#define MIN_STACK_BOUNDARY 128
+
 #undef MAIN_STACK_BOUNDARY
 #define MAIN_STACK_BOUNDARY 128
 
The problem begins with...


	  /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/./gcc/xgcc -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/./gcc/ -B/sw/lib/gcc4.6/x86_64-apple-darwin10.5.0/bin/ -B/sw/lib/gcc4.6/x86_64-apple-darwin10.5.0/lib/ -isystem /sw/lib/gcc4.6/x86_64-apple-darwin10.5.0/include -isystem /sw/lib/gcc4.6/x86_64-apple-darwin10.5.0/sys-include    -g -O2 -O2  -g -O2 -DIN_GCC   -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include  -fPIC -pipe -g -DHAVE_GTHR_DEFAULT -DIN_LIBGCC2 -D__GCC_FLOAT_NOT_NEEDED   -I. -I. -I../.././gcc -I../../../gcc-4.6-20100830/libgcc -I../../../gcc-4.6-20100830/libgcc/. -I../../../gcc-4.6-20100830/libgcc/../gcc -I../../../gcc-4.6-20100830/libgcc/../include  -DHAVE_CC_TLS -DUSE_EMUTLS -fvisibility=hidden -DHIDE_EXPORTS -c eh_dummy.c		\
	     -o eh_dummy.o;				\
	  objects=eh_dummy.o;				\
	fi;							\
	ar  rc libgcc.a $objects
/usr/bin/nm: no name list
...

showing that MIN_STACK_BOUNDARY is insufficient to maintain alignment on system calls.
Comment 5 Jack Howarth 2010-08-31 01:23:28 UTC
Created attachment 21598 [details]
patch to properly use MIN_STACK_BOUNDARY on intel darwin.
Comment 6 Jack Howarth 2010-08-31 01:25:58 UTC
For the testcase compiled with...

gcc-4 -m32 -O -fomit-frame-pointer -fno-pic -S add.c

...current gcc trunk gives...


_f:
LFB0:
        subl    $12, %esp
LCFI0:
        addl    $1, _a
        addl    $12, %esp
LCFI1:
        ret

With patch we now get...


_f:
LFB0:
        addl    $1, _a
        ret
Comment 7 H.J. Lu 2010-08-31 02:18:40 UTC
Please check how MIN_STACK_BOUNDARY is used in i386.c. If you
define it to MIN_STACK_BOUNDARY, -mstackrealign won't work
correctly.
Comment 8 H.J. Lu 2010-08-31 02:21:32 UTC
(In reply to comment #7)
> Please check how MIN_STACK_BOUNDARY is used in i386.c. If you
> define it to MIN_STACK_BOUNDARY, -mstackrealign won't work
> correctly.
> 

I meant -mstackrealign won't work correctly if MIN_STACK_BOUNDARY
is 128.
Comment 9 Jack Howarth 2010-08-31 05:13:27 UTC
Created attachment 21599 [details]
rely on PREFERRED_STACK_BOUNDARY_DEFAULT for intel darwin
Comment 10 Jack Howarth 2010-08-31 05:15:39 UTC
On reflection, we should be able to dump all the stack related setting in darwin.h and use the current setting of PREFERRED_STACK_BOUNDARY_DEFAULT 128 since current gcc now defaults on SSE2.
Comment 11 Jack Howarth 2010-08-31 07:04:24 UTC
Using the proposed PR36502v2.patch, which eliminates any attempts to change the default stack boundary handling in gcc trunk, I find the following regressions at -m32 on x86_64-apple-darwin10...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal compiler error)
FAIL: gcc.dg/nest.c execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test

while eliminating PR36502 miscompilation of the add.c test case.
Comment 12 Jack Howarth 2010-08-31 12:57:48 UTC
Testresults for PR36502v2.patch at http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg03098.html. It appears that the only additional failures triggered by this patch are listed in comment 11.
Comment 13 H.J. Lu 2010-08-31 13:48:08 UTC
(In reply to comment #9)
> Created an attachment (id=21599) [edit]
> rely on PREFERRED_STACK_BOUNDARY_DEFAULT for intel darwin
> 

You should keep

#undef MAIN_STACK_BOUNDARY
#define MAIN_STACK_BOUNDARY 128
Comment 14 H.J. Lu 2010-08-31 13:54:28 UTC
(In reply to comment #12)
> Testresults for PR36502v2.patch at
> http://gcc.gnu.org/ml/gcc-testresults/2010-08/msg03098.html. It appears that
> the only additional failures triggered by this patch are listed in comment 11.
> 

Please try this patch:

http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01916.html
Comment 15 Jack Howarth 2010-08-31 15:39:03 UTC
Created attachment 21607 [details]
rely on PREFERRED_STACK_BOUNDARY_DEFAULT and MAIN_STACK_BOUNDARY
Comment 16 Jack Howarth 2010-08-31 15:48:14 UTC
(In reply to comment #14)
> Please try this patch:
> 
> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01916.html
> 

With the patch above and PR36502v3.patch, I still get...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal compiler error)

which appears as...

Executing on host: /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.c-torture/execute/builtins/sprintf-chk.c /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.c-torture/execute/builtins/sprintf-chk-lib.c /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.c-torture/execute/builtins/lib/main.c  -w  -Os   -lm   -m32 -o /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/testsuite/gcc/sprintf-chk.x7    (timeout = 300)
/sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.c-torture/execute/builtins/sprintf-chk.c:197:1: internal compiler error: in div_data_align, at dwarf2out.c:595

Interestingly, if I run...

/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/cc1 -quiet -v -imultilib i386 -iprefix /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/../lib/gcc/x86_64-apple-darwin10.5.0/4.6.0/ -isystem /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/include -isystem /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/include-fixed -D__DYNAMIC__ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.c-torture/execute/builtins/sprintf-chk.c -fPIC -quiet -dumpbase sprintf-chk.c -mmacosx-version-min=10.6.5 -m32 -mtune=generic -auxbase sprintf-chk -Os -w -version -o /var/tmp//ccVmszmK.s

(obtained with -v) through gdb, the crash is suppressed and the result sprintf-chk.x7 binary executes fine.
Comment 17 Jack Howarth 2010-08-31 16:30:38 UTC
With http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01916.html and the patch in comment 15, I am  also still failing at -m32 on x86_64-apple-darwin10...

FAIL: gcc.dg/nest.c execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -Os  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -flto  execution test
FAIL: gcc.dg/torture/stackalign/alloca-4.c  -O2 -fwhopr  execution test

as well as a new additional failure of...

FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\t48\tdynamic,bounded

which wasn't present with just the patch from comment 11. 
Comment 18 Jack Howarth 2010-08-31 16:36:03 UTC
Created attachment 21608 [details]
assembly from failing gcc.dg/nest.c execution test at -m32 on x86_64-apple-darwin10

Generated with...

/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.dg/nest.c -O2 -pg -lm -m32 --save-temps -o ./nest.exe

using a gcc built with the patches from comments 14 and 15.
Comment 19 Jack Howarth 2010-08-31 16:41:00 UTC
(In reply to comment #18)

In gdb, the failing nest.exe exection shows...


Starting program: /Users/howarth/stack_align_bug2/nest.exe 
Reading symbols for shared libraries ++. done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: 13 at address: 0x00000000
0x931e6f30 in misaligned_stack_error_ ()
(gdb) bt
#0  0x931e6f30 in misaligned_stack_error_ ()
#1  0x93293cc4 in monstartup ()
#2  0x00001e9e in main ()
Comment 20 Jack Howarth 2010-08-31 16:42:24 UTC
Created attachment 21609 [details]
assembly from failing gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test at -m32 on x86_64-apple-darwin10
Comment 21 Jack Howarth 2010-08-31 16:46:11 UTC
(In reply to comment #20)

Created alloca-4.s with...

 /sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/xgcc -B/sw/src/fink.build/gcc46-4.6.0-1000/darwin_objdir/gcc/ /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.dg/torture/stackalign/alloca-4.c -O1 -m32 -mincoming-stack-boundary=2 -mpreferred-stack-boundary=2 -lm -m32 --save-temps -o ./alloca-4.exe

against patches from comments 14 and 15. The test case backtraces in gdb as...

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: 13 at address: 0x00000000
0x931e6f30 in misaligned_stack_error_ ()
(gdb) bt
#0  0x931e6f30 in misaligned_stack_error_ ()
#1  0x8fe0c582 in __dyld__ZL9commatizeyPc ()
#2  0x00001db5 in bar (p=0xbffff340 "", size=5) at /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.dg/torture/stackalign/alloca-4.c:10
#3  0x00001e7b in main () at /sw/src/fink.build/gcc46-4.6.0-1000/gcc-4.6-20100830/gcc/testsuite/gcc.dg/torture/stackalign/alloca-4.c:38
Comment 22 Jack Howarth 2010-08-31 19:50:32 UTC
Created attachment 21615 [details]
assembly from stock build of gcc.dg/nest.c execution test at -m32 on x86_64-apple-darwin10
Comment 23 Jack Howarth 2010-08-31 19:55:47 UTC
Created attachment 21616 [details]
assembly from stock build of gcc.dg/torture/stackalign/alloca-4.c  -O1  execution test at -m32 on x86_64-apple-darwin10
Comment 24 Jack Howarth 2010-08-31 19:57:33 UTC
Created attachment 21617 [details]
diff between nest.s.stock and nest.s show alignment changes in failing test case
Comment 25 Jack Howarth 2010-08-31 19:59:15 UTC
Created attachment 21618 [details]
diff between alloca-4.s.stock and alloca-4.s show alignment changes in failing test case
Comment 26 H.J. Lu 2010-08-31 20:30:39 UTC
(In reply to comment #15)
> Created an attachment (id=21607) [edit]
> rely on PREFERRED_STACK_BOUNDARY_DEFAULT and MAIN_STACK_BOUNDARY
> 

This is wrong:

-/* Since we'll never want a stack boundary less aligned than 128 bits
-   we need the extra work here otherwise bits of gcc get very grumpy
-   when we ask for lower alignment.  We could just reject values less
-   than 128 bits for Darwin, but it's easier to up the alignment if
-   it's below the minimum.  */
-#undef PREFERRED_STACK_BOUNDARY
-#define PREFERRED_STACK_BOUNDARY			\
-  MAX (STACK_BOUNDARY, ix86_preferred_stack_boundary)
-

You must keep it and replace STACK_BOUNDARY with 128.
Comment 27 mrs@gcc.gnu.org 2010-09-01 00:25:36 UTC
I don't think MAIN_STACK_BOUNDARY needs to be set, nor will it help to set it.  The alignment is set up by the crt runtime, and just to call to main, the alignment of the stack must be 128, before we issue the call.
Comment 28 H.J. Lu 2010-09-01 01:08:58 UTC
(In reply to comment #27)
> I don't think MAIN_STACK_BOUNDARY needs to be set, nor will it help to set it. 
> The alignment is set up by the crt runtime, and just to call to main, the
> alignment of the stack must be 128, before we issue the call.
> 

If you don't set MAIN_STACK_BOUNDARY to 128, gcc may
align stack in main.
Comment 29 Jack Howarth 2010-09-01 04:02:58 UTC
(In reply to comment #28)
> If you don't set MAIN_STACK_BOUNDARY to 128, gcc may
> align stack in main.
> 

I am seeing some instability in the testsuite results when I don't redefine MAIN_STACK_BOUNDARY to 128. In the first run, I got two regressions...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal compiler error)
FAIL: gcc.dg/nest.c execution test

and in the second I only got...

FAIL: gcc.dg/nest.c execution test

which is a common regression with and without redefining MAIN_STACK_BOUNDARY to 128.
Comment 30 Jack Howarth 2010-09-01 05:34:24 UTC
With only gcc-pr45234-2.patch at r163712 , I am seeing the following regressions...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal compiler error)
FAIL: gcc.dg/stack-usage-1.c scan-file foo\t(256|264)\tstatic
FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\t48\tdynamic,bounded

whereas with gcc-pr45234-2.patch and...

Index: gcc/config/i386/darwin.h
===================================================================
--- gcc/config/i386/darwin.h    (revision 163704)
+++ gcc/config/i386/darwin.h    (working copy)
@@ -78,9 +78,6 @@
 /* On Darwin, the stack is 128-bit aligned at the point of every call.
    Failure to ensure this will lead to a crash in the system libraries
    or dynamic loader.  */
-#undef STACK_BOUNDARY
-#define STACK_BOUNDARY 128
-
 #undef MAIN_STACK_BOUNDARY
 #define MAIN_STACK_BOUNDARY 128
 
@@ -91,7 +88,7 @@
    it's below the minimum.  */
 #undef PREFERRED_STACK_BOUNDARY
 #define PREFERRED_STACK_BOUNDARY                       \
-  MAX (STACK_BOUNDARY, ix86_preferred_stack_boundary)
+  MAX (128, ix86_preferred_stack_boundary)
 
 /* We want -fPIC by default, unless we're using -static to compile for
    the kernel or some such.  */

I see the regressions...

FAIL: gcc.dg/nest.c execution test
FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\t48\tdynamic,bounded
FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\t48\tdynamic,bounded

suggesting that the nest.c failure is the only one introduced by the darwin.h changes
and the remainder are due to gcc-pr45234-2.patch.
Comment 31 H.J. Lu 2010-09-01 14:51:44 UTC
(In reply to comment #30)
> With only gcc-pr45234-2.patch at r163712 , I am seeing the following
> regressions...
> 
> FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal
> compiler error)

Please provide preprocessed .i file.

> FAIL: gcc.dg/stack-usage-1.c scan-file foo\t(256|264)\tstatic
> FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\t48\tdynamic,bounded
> 
> whereas with gcc-pr45234-2.patch and...

I compared revision 163733 against revision 163733 + gcc-pr45234-2.patch
with a cross compiler to x86_64-apple-darwin10.4.0.  I saw no
differences in .su files for gcc.dg/stack-usage-1.c nor
gcc.target/i386/stack-usage-realign.c.  Are you sure they pass
on Darwin with revision 163733?
Comment 32 Jack Howarth 2010-09-01 15:18:19 UTC
(In reply to comment #31)

> I compared revision 163733 against revision 163733 + gcc-pr45234-2.patch
> with a cross compiler to x86_64-apple-darwin10.4.0.  I saw no
> differences in .su files for gcc.dg/stack-usage-1.c nor
> gcc.target/i386/stack-usage-realign.c.  Are you sure they pass
> on Darwin with revision 163733?
> 

HJ,
   Actually it appears that...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal compiler error)
UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os 
FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
FAIL: gcc.target/i386/stack-usage-realign.c scan-file main\\t48\\tdynamic,bounded

are actually new regressions present in unpatched gcc trunk...

http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00042.html

I will do a regression hunt later today to find out the offending commit.
Comment 33 Jack Howarth 2010-09-01 15:30:09 UTC
Created attachment 21647 [details]
Don't redefine STACK_BOUNDARY and replace STACK_BOUNDARY with 128.
Comment 34 H.J. Lu 2010-09-01 15:31:37 UTC
(In reply to comment #33)
> Created an attachment (id=21647) [edit]
> Don't redefine STACK_BOUNDARY and replace STACK_BOUNDARY with 128.
> 

Please do a proper regression test and report REAL regressions.
Comment 35 Jack Howarth 2010-09-01 15:37:42 UTC
(In reply to comment #34)
> Please do a proper regression test and report REAL regressions.
> 

First we need to do a regression hunt in gcc trunk for the new
stack test case failures. It is impossible to properly test the
patch for PR36502 against a gcc trunk which is already regressed.
Comment 36 Jack Howarth 2010-09-01 17:45:01 UTC
FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal
compiler error)
UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os 
FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
FAIL: gcc.target/i386/stack-usage-realign.c scan-file
main\\t48\\tdynamic,bounded

are all due to r163660 and present up to at least r163712. A regtest of r163659 with gcc-pr45234-2.patch and PR36502v4.patch only shows only a single new regression...

FAIL: gcc.dg/nest.c execution test

which in gdb appears as...

(gdb) r
Starting program: /Users/howarth/nest_bug/nest.exe 
Reading symbols for shared libraries ++. done

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: 13 at address: 0x00000000
0x978acef0 in misaligned_stack_error_ ()

the failure disappears if -pg is removed from the compilation of nest.exe.

Will try r163659 with only gcc-pr45234-2.patch next.
Comment 37 Jack Howarth 2010-09-01 18:01:25 UTC
The nest.c failure doesn't occur with r163659 and gcc-pr45234-2.patch. So PR36502v4.patch solves PR36502 but introduces a single regression in the gcc testsuite.
Comment 38 H.J. Lu 2010-09-01 19:49:34 UTC
Created attachment 21649 [details]
A patrch

Darwin needs 128bit stack boundary for all function calls.  This
patch should fix nest.c.
Comment 39 H.J. Lu 2010-09-01 20:03:29 UTC
Created attachment 21650 [details]
Another patch

Darwin needs 128bit stack alignment for -pg.
Comment 40 Jack Howarth 2010-09-01 21:28:17 UTC
(In reply to comment #39)
> Created an attachment (id=21650) [edit]
> Another patch
> 
> Darwin needs 128bit stack alignment for -pg.
> 

This second patch (which is limited to gcc/config/i386/darwin.h) eliminates both PR36502 and the nest.c failures on x86_64-apple-darwin10 at -m32. We still get random failures in...

FAIL: gcc.c-torture/execute/builtins/sprintf-chk.c compilation,  -Os  (internal
compiler error)
UNRESOLVED: gcc.c-torture/execute/builtins/sprintf-chk.c execution,  -Os 
FAIL: gcc.dg/stack-usage-1.c scan-file foo\\t(256|264)\\tstatic
FAIL: gcc.target/i386/stack-usage-realign.c scan-file
main\\t48\\tdynamic,bounded

but that the fault of r163660.
Comment 41 Jack Howarth 2010-09-03 01:58:05 UTC
Created attachment 21681 [details]
don't redefine MAIN_STACK_BOUNDARY so -fstack-usage can work
Comment 42 Jack Howarth 2010-09-04 20:39:56 UTC
Posted final version of proposed patch to...

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

This patch also fixes PR42313 and PR44651 as yet another added bonus.
Comment 43 Jack Howarth 2010-09-04 21:21:36 UTC
Updated patch to reflect the wider coverage of PRs fixed...
http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00365.html
Comment 44 Jack Howarth 2010-09-06 13:17:57 UTC
Created attachment 21709 [details]
retain redefinition of MAIN_STACK_BOUNDARY as required
Comment 45 Jack Howarth 2010-09-06 13:57:40 UTC
(In reply to comment #44)
> Created an attachment (id=21709) [edit]
> retain redefinition of MAIN_STACK_BOUNDARY as required
> 

Testsuite results for the PR36502v9.patch are shown in http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00537.html. The compiler ICEs shown for gfortran.dg/backspace_1.f, gfortran.dg/record_marker_2.f, gfortran.dg/graphite/pr42393-1.f90 libgomp.fortran/appendix-a/a.16.1.f90, libgomp.fortran/omp_atomic2.f90, libgomp.graphite/force-parallel-3.c, libgomp.graphite/force-parallel-9.c and 25_algorithms/heap/moveable.cc shouldn't be due to my patch as the identical patch (except for test cases corrections) was tested in http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00168.html and didn't show them. While I don't see these in other reported i386-apple-darwin10 testresults, those aren't using --enable-checking=yes. I will rebuild gcc trunk without PR36502v9.patch, reconfirm those ICEs and file PRs against them separately.
Comment 46 Dominique d'Humieres 2010-09-06 14:04:51 UTC
> gfortran.dg/backspace_1.f, gfortran.dg/record_marker_2.f, ...

They are pr45534 and probably fixed at revision 163913 (testing).
Comment 47 Jack Howarth 2010-09-07 03:27:30 UTC
Testsuite results for gcc-pr45234-2.patch and PR36502v9.patch on i386-apple-darwin10 are at...

http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00603.html

PR36502, PR42313 and PR44651 fixed as well as stack realignment, when not on main,  enabled  (as shown by the new and revised test cases passing). No new regressions in test results at either -m32 or -m64.
Comment 48 hjl@gcc.gnu.org 2010-09-07 21:19:20 UTC
Subject: Bug 36502

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 49 H.J. Lu 2010-09-08 02:16:15 UTC
Fixed.
Comment 50 hjl@gcc.gnu.org 2010-09-10 16:13:10 UTC
Subject: Bug 36502

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