From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8270 invoked by alias); 18 Jan 2013 13:14:12 -0000 Received: (qmail 8233 invoked by uid 22791); 18 Jan 2013 13:14:11 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO,TW_EG X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Jan 2013 13:14:05 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id C7FA11C7C38; Fri, 18 Jan 2013 08:14:04 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id L2deHnxaRNT9; Fri, 18 Jan 2013 08:14:04 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 433341C7BD1; Fri, 18 Jan 2013 08:14:04 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 7B5B9C3EEF; Fri, 18 Jan 2013 17:13:58 +0400 (RET) Date: Fri, 18 Jan 2013 13:14:00 -0000 From: Joel Brobecker To: Jiong Wang Cc: gdb-patches@sourceware.org, Walter Lee Subject: Re: [RFC/TileGX 1/6] fix args alignment bug Message-ID: <20130118131358.GE3564@adacore.com> References: <50F9148F.3010602@tilera.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50F9148F.3010602@tilera.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2013-01/txt/msg00415.txt.bz2 > gdb/ChangeLog: > > * tilegx-tdep.c (tilegx_push_dummy_call): args pushed on stack > should be aligned to 64bit. Some questions below... > - /* Loop backwards through arguments to determine stack alignment. */ > - alignlen = 0; > - > - for (j = nargs - 1; j >= i; j--) > - { > - typelen = TYPE_LENGTH (value_enclosing_type (args[j])); > - alignlen += (typelen + 3) & (~3); > - } > - > - if (alignlen & 0x4) > - stack_dest -= 4; Do you have any hint as to why this code was written the way it was? It doesn't seem like the type of code that would be added by accident. Did Jeff (the apparent author) misunderstand the ABI? > typelen = TYPE_LENGTH (value_enclosing_type (args[j])); > - slacklen = ((typelen + 3) & (~3)) - typelen; > + slacklen = ((typelen + 7) & (~7)) - typelen; This is a detail, but can you use utils.c:align_up, in this case? (I find those midly more readable than these incantations, but it's OK if you prefer the code to stay as is). > /* Add 2 words for linkage space to the stack. */ > - stack_dest = stack_dest - 8; > - write_memory (stack_dest, two_zero_words, 8); > + stack_dest = stack_dest - 16; > + write_memory (stack_dest, four_zero_words, 16); It looks like you need to adjust the comment as well. -- Joel