[patch i386]: For 4.6 add support of thiscall calling convention attribute for x86

Kai Tietz ktietz70@googlemail.com
Sat Mar 20 12:35:00 GMT 2010


2010/3/20 Dave Korn <dave.korn.cygwin@googlemail.com>:
>> Tested for i686-pc-mingw32, i686-pc-cygwin, and x86_64-pc-mingw32. It
>> needs review by an i386 maintainer and if there are no objections I
>> want to apply it as soon as 4.6 is open for commit.
>
>  Couple of nits, no objections:
>
>> +      if (lookup_attribute ("thiscall", TYPE_ATTRIBUTES (*node)))
>> +        {
>> +       error ("fastcall and thiscall attributes are not compatible");
>> +     }
>
>  You have a space-vs-TAB problem everywhere you've cut and pasted this construct.
>
>>        if (lookup_attribute ("fastcall", TYPE_ATTRIBUTES (type)))
>>       regno = aggr ? DX_REG : CX_REG;
>> +      else if (lookup_attribute ("thiscall", TYPE_ATTRIBUTES (type)))
>> +     regno = aggr ? DX_REG : CX_REG;
>>        else
>
>  That's a bit cut-n-pastey, how about one if that ORs the two conditions and
> only one assignment?  (As I see you did it below in x86_output_mi_thunk.)

Well, duplicated it here by intention. I am not absolutely sure here,
if description provided by MS documentation covers here the same
behavior as for stdcall when aggregate is set.

>> +If the number of arguments is variable all arguments are pushed on the
>> +stack.
>
>  There don't appear to be any stdargs function calls in the tests you added?
>  It sounds like the kind of potentially-buggy corner case that could really
> use a test just to make sure it stays working.

Well, this is a second item to be decided. By MS description are
stdarg-functions not allowed for thiscall (btw as for fastcall), but
gcc is able to handle them. Should we use current behavior of gcc for
this, or should error/warn for stdarg functions using
thiscall/fastcall?

Chees,
Kai



-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination



More information about the Gcc-patches mailing list