Re: [RFA Darwin]: Add push_dummy_call for i386

[Tristan Gingold <gingold at adacore dot com>]

On Jul 3, 2009, at 6:48 PM, Mark Kettenis wrote:

> > Date: Fri, 3 Jul 2009 17:51:52 +0200
> > From: Tristan Gingold <gingold@adacore.com>
> >
> > Hi,
> >
> > Darwin i386 ABI is slightly different from the SVR4 one.  In  
> > particular
> > stack alignment is 16.  As a consquence, i386 Darwin can't use the  
> > standard
> > i386-tdeo.c push_dummy_call and this patch provides a Darwin  
> > version of this
> > call.
> >
> > Regtested on i386 Darwin.
> > Tristan.
>
> Tristan, can you provide unified diffs instead of context diffs?   
> It's much easier to read the unified ones.

Sure, I can ;-)

> > + static inline int
> > + i386_m128_p (struct type *type)
> > + {
> > +   return TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)
> > +     && TYPE_LENGTH (type) == 16;
> > + }
>
> Any reason why this function must be inline?  Ever since the bright
> folks in the ISO committee decided to adpot different rules for things
> like static inline and extern inline than GCC, the use of inline makes
> me nervous.  And I believe modern compilers are very well capable
> themselves of deciding if static functions should be inlined or not.

I will remove the inline.

> > + /* Check whether TYPE must be 16-byte-aligned when passed as a
> > +    function argument.  16-byte vectors, _Decimal128 and  
> > structures or
> > +    unions containing such types must be 16-byte-aligned; other
> > +    arguments are 4-byte-aligned.  */
>
> Hmm, this function actually returns the alignment (as a number of
> bytes), but the comment suggests it's a predicate.  Can you adjust the
> comment?

Sure.

> > + static int
> > + i386_darwin_arg_type_alignment (struct type *type)
> > + {
> > +   type = check_typedef (type);
> > +   /* Passing arguments.
> > +      5.  The caller places 64-bit vectors (__m64) on the  
> > parameter area,
> > +          aligned to 8-byte boundaries.
>
> "on the parameter area"?  Probably a type.

I quoted LowLevelABI.pdf.  What is wrong in this sentence ?  (Keep in  
mind that I am not a native
english speaker!)

> > +   for (write_pass = 0; write_pass < 2; write_pass++)
> > +     {
> > +       int args_space = 0;
> > +       int nbr_m128 = 0;
>
> nbr_m128?  Is that supposed to mean number of m128's?  If so, would
> you be so kind to reanem this variable num_m128?

Ok.

> > *************** i386_darwin_init_abi (struct gdbarch_inf
> > *** 127,133 ****
> >   tdep->sc_reg_offset = i386_darwin_thread_state_reg_offset;
> >   tdep->sc_num_regs = i386_darwin_thread_state_num_regs;
> >
> > !   tdep->jb_pc_offset = 20;
> >
> >   set_solib_ops (gdbarch, &darwin_so_ops);
> > }
> > --- 255,266 ----
> >   tdep->sc_reg_offset = i386_darwin_thread_state_reg_offset;
> >   tdep->sc_num_regs = i386_darwin_thread_state_num_regs;
> >
> > !   tdep->jb_pc_offset = 48;
> > !
> > !   /* Although the i387 extended floating-point has only 80  
> > significant
> > !      bits, a `long double' actually takes up 128, probably to  
> > enforce
> > !      alignment.  */
> > !   set_gdbarch_long_double_bit (gdbarch, 128);
>
> This is true for the 32-bit Darwin ABI as well?

This is true only for the i386 Darwin ABI.  Darwin strictly follows  
the SVR4 ABI for x86_64.

Tristan.