[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