[patch i386]: For 4.6 add support of thiscall calling convention attribute for x86
Dave Korn
dave.korn.cygwin@googlemail.com
Sat Mar 20 12:35:00 GMT 2010
> 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.)
> +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.
cheers,
DaveK
More information about the Gcc-patches
mailing list