From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2208 invoked by alias); 13 Jun 2012 21:20:12 -0000 Received: (qmail 2198 invoked by uid 22791); 13 Jun 2012 21:20:10 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 13 Jun 2012 21:19:57 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id q5DLJqbh013658; Wed, 13 Jun 2012 23:19:52 +0200 (CEST) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id q5DLJSI4003388; Wed, 13 Jun 2012 23:19:28 +0200 (CEST) Date: Wed, 13 Jun 2012 21:20:00 -0000 Message-Id: <201206132119.q5DLJSI4003388@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: jan.kratochvil@redhat.com CC: gdb-patches@sourceware.org In-reply-to: <20120613155020.GB26214@host2.jankratochvil.net> (message from Jan Kratochvil on Wed, 13 Jun 2012 17:50:20 +0200) Subject: Re: [patch 2/3] Align stack for SSE (PR tdep/14222) References: <20120613155020.GB26214@host2.jankratochvil.net> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-06/txt/msg00440.txt.bz2 > Date: Wed, 13 Jun 2012 17:50:20 +0200 > From: Jan Kratochvil > > Hi, > > this is mostly independent patch. But it does not make sense to discuss > alignment in [patch 3/3] when it has been already broken. > > http://groups.google.com/group/ia32-abi/browse_thread/thread/4f9b3e5069943bf1 > says that gcc-4.6 aligns stack to 32 bytes. I do not see it anywhere: > 80482e8: 83 e4 f0 and $0xfffffff0,%esp > It is aligned just to 16 bytes. Also checked that Intel CPU documentation > does not require any more alignment than 16 bytes (sure it may be less > effective but traps never occur). Therefore I consider amd64-tdep.c to be > correct and I have not changed its existing 16-bytes alignment. > > gdb/ > 2012-06-13 Jan Kratochvil > > PR tdep/14222 > * i386-tdep.c (i386_push_dummy_call): Align to 16 bytes unconditionally. Committed a better diff (see below) that removes the now unused have_16_byte_aligned_arg variable. > gdb/testsuite/ > 2012-06-13 Jan Kratochvil > > PR tdep/14222 > * gdb.arch/i386-sse-stack-align.S: New file. > * gdb.arch/i386-sse-stack-align.c: New file. > * gdb.arch/i386-sse-stack-align.exp: New file. Can you commit this testsuite bit? 2012-06-13 Mark Kettenis Jan Kratochvil PR tdep/14222 * i386-tdep.c (i386_push_dummy_call): Unconditionally align the stack on a 16-byte boundary. Index: i386-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/i386-tdep.c,v retrieving revision 1.352 diff -u -p -r1.352 i386-tdep.c --- i386-tdep.c 13 Jun 2012 20:29:15 -0000 1.352 +++ i386-tdep.c 13 Jun 2012 21:15:44 -0000 @@ -2351,7 +2351,6 @@ i386_push_dummy_call (struct gdbarch *gd for (write_pass = 0; write_pass < 2; write_pass++) { int args_space_used = 0; - int have_16_byte_aligned_arg = 0; if (struct_return) { @@ -2389,19 +2388,20 @@ i386_push_dummy_call (struct gdbarch *gd else { if (i386_16_byte_align_p (value_enclosing_type (args[i]))) - { - args_space = align_up (args_space, 16); - have_16_byte_aligned_arg = 1; - } + args_space = align_up (args_space, 16); args_space += align_up (len, 4); } } if (!write_pass) { - if (have_16_byte_aligned_arg) - args_space = align_up (args_space, 16); sp -= args_space; + + /* The original System V ABI only requires word alignment, + but modern incarnations need 16-byte alignment in order + to support SSE. Since wasting a few bytes here isn't + harmful we unconditionally enforce 16-byte alignment. */ + sp &= ~0xf; } }