Re: [RFA Darwin]: Add push_dummy_call for i386

[Tristan Gingold <gingold@adacore.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.