Re: [RFA] sh-tdep.c (sh_use_struct_convention): Restructure and fix

[Elena Zannoni <ezannoni@redhat.com>]

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.

 > 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. 

 > Corinna
 > 
 > 	* sh-tdep.c (sh_use_struct_convention): Clean up to have a
 > 	more readable code.  Accomodate 4 byte structs with 4 byte sized
 > 	first field (e.g. bitfields).
 > 
 > Index: sh-tdep.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/sh-tdep.c,v
 > retrieving revision 1.145
 > diff -u -p -r1.145 sh-tdep.c
 > --- sh-tdep.c	3 Oct 2003 08:13:37 -0000	1.145
 > +++ sh-tdep.c	4 Oct 2003 11:32:00 -0000
 > @@ -565,8 +565,25 @@ sh_use_struct_convention (int gcc_p, str
 >  {
 >    int len = TYPE_LENGTH (type);
 >    int nelem = TYPE_NFIELDS (type);
 > -  return ((len != 1 && len != 2 && len != 4 && len != 8) || nelem != 1) &&
 > -    (len != 8 || TYPE_LENGTH (TYPE_FIELD_TYPE (type, 0)) != 4);
 > +
 > +  /* Non-power of 2 length types and types bigger than 8 bytes (which don't
 > +     fit in two registers anyway) use struct convention.  */
 > +  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?
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?


 > +	return 1;

 > +      /* ... 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.


elena


 > +	return 1;
 > +    }
 > +  /* Otherwise return in registers.  */
 > +  return 0;
 >  }
 >  
 >  /* Extract from an array REGBUF containing the (raw) register state
 > 
 > -- 
 > Corinna Vinschen
 > Cygwin Developer
 > Red Hat, Inc.