From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5133 invoked by alias); 10 Oct 2003 16:28:08 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 5103 invoked from network); 10 Oct 2003 16:28:07 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 10 Oct 2003 16:28:07 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h9AGS6M21328 for ; Fri, 10 Oct 2003 12:28:06 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h9AGS6c02301 for ; Fri, 10 Oct 2003 12:28:06 -0400 Received: from localhost.redhat.com (devserv.devel.redhat.com [172.16.58.1]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id h9AGS595008548 for ; Fri, 10 Oct 2003 12:28:06 -0400 Received: by localhost.redhat.com (Postfix, from userid 469) id E9EA32CCB5; Fri, 10 Oct 2003 12:39:16 -0400 (EDT) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <16262.57524.404285.995903@localhost.redhat.com> Date: Fri, 10 Oct 2003 16:28:00 -0000 To: gdb-patches@sources.redhat.com Subject: Re: [RFA] sh-tdep.c (sh_use_struct_convention): Restructure and fix In-Reply-To: <20031010072929.GF14344@cygbert.vinschen.de> References: <20031004113939.GK11435@cygbert.vinschen.de> <16261.53197.951881.194874@localhost.redhat.com> <20031010072929.GF14344@cygbert.vinschen.de> X-SW-Source: 2003-10/txt/msg00359.txt.bz2 Corinna Vinschen writes: > On Thu, Oct 09, 2003 at 05:14:53PM -0400, Elena Zannoni wrote: > > Corinna Vinschen writes: > > > Hi, > > > > > > the below patch straightens out sh_use_struct_convention() so that it > > > allows a far better readability than before, especially by allowing > > > a bunch of comments spread out through the code. > > > > > > > I just added a detailed comment. Does that match what you implemented? > > I'd prefer the use of 'aggregate' instead of 'struct' in your comments. > > Yes, thanks, I saw the comment. It's enlightening. However, the > first sentence seems to be a copy/paste hangover: > > /* Should call_function allocate stack space for a struct return? > > And even though I have to admit, that I'm not 100% sure (perhaps > I miss a case) I think the implementation should match at least 99% > of the description. > > The difference between the old and the new code is given by allowing > 4 byte structs (erm, aggregates) with more than one element, but a size > of 4 byte for the first element. This sounds somewhat weird, but that's > exactly the case if the 4 byte agregate is a bitfield or contains a > bitfield. So the change in this patch solves exactly these bitfields > as return type problem. > > > > Additionally it fixes one bug: A struct of lenght 4 bytes, which > > > consists of only a bitfield, is returned in register r0, not on the > > > stack using the struct convention. So far, GDB got that wrong. > > > > Is there a test case that was failing? If not, it should be added. > > Yes, testcases which uncover that problem exist in call-ar-st and > call-rt-st. > > Actually the whole change was a result of these testcases. I saw the > bitfield problem but I found the former one-expression evaluation > very unreadable. So, first I straightened out the expression, then > I added the bitfield case. > > > > + if (len != 1 && len != 2 && len != 4 && len != 8) > > > + return 1; > > > + /* Structs with more than 1 fields use struct convention, if... */ > > > + if (nelem != 1) > > > + { > > > + /* ... they are 1 or 2 bytes in size (e.g. struct of two chars)... */ > > > + if (len != 4 && len != 8) > > > > Can you just say len == 1 or len == 2 so that it matches your comment? > > Sure. No problem to change this. > > > Wait, this contradicts what the comments I just added say: > > > > For example, a 2-byte aligned structure with size 2 bytes has the > > same size and alignment as a short int, and will be returned in R0. > > > > Which is correct? > > Both. An aggregate of size 1 or 2 byte with more than 1 element is not > correctly alligned so it will not be returned in R0. > How does this get returned? struct {char a; char b;} Shouldn't this be in R0 with some padding? You code would return it in memory. elena > > > + /* ... or, if the struct is 4 or 8 bytes and the first field is > > > + not of size 4 bytes. Note that this also covers structs with > > > + bitfields. */ > > > + if (TYPE_LENGTH (TYPE_FIELD_TYPE (type, 0)) != 4) > > > > I am not sure I understand this one, that's why asked a pointer to a > > test case. It seems to contradict the following, i.e. it should still > > be in registers, or maybe I don't understand the language: > > > > When an aggregate type is returned in R0 and R1, R0 contains the first > > four bytes of the aggregate, and R1 contains the remainder. If the size > > of the aggregate type is not a multiple of 4 bytes, the aggregate is > > tail-padded up to a multiple of 4 bytes. The value of the padding is > > undefined. > > Is my above description better? The code is identical to the former > implementation except for the 4 byte bitfield case. That one is now > covered here. > > I have not attached a new patch, but I've noted to change "struct" to > "aggregate" in the comments and the > if (len != 4 && len != 8) > to > if (len == 1 || len == 2) > > Corinna > > -- > Corinna Vinschen > Cygwin Developer > Red Hat, Inc.