[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