Re: [RFA Darwin]: Add push_dummy_call for i386

Tristan Gingold
2009-07-06T07:08:43Z

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.