This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch i386]: For 4.6 add support of thiscall calling convention attribute for x86
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