Testing  a patch to add a new switch for windows targets.

Patch submitted
http://gcc.gnu.org/ml/gcc-patches/2008-07/msg01250.html
Danny

Thank you. Should the option be enabled by default on i686-pc-mingw32? Perhaps not, but it does make good on a previously broken ABI promise.

The test case in that patch only covers the callee, not the caller. The difference in the calling code is perhaps less easily recognisable.

I'd be willing to offer a small bounty for this one.  Maybe $50 for an accepted patch that handles both the caller and callee sides?

The problem seems to be common [1].

Thanks!
-=r
[1] http://209.85.173.132/search?q=cache:e7XCjhLwHacJ:luabinaries.luaforge.net/manual.html+http://luabinaries.luaforge.net/manual.html%23LuaBinariesCompatible&hl=en&ct=clnk&cd=1&gl=us&client=opera is it the same bug [they mention incompats when passing CRT objects among libraries]? dunno.

(In reply to comment #4)

>http://209.85.173.132/search?q=cache:e7XCjhLwHacJ:luabinaries.luaforge.net/manual.html+http://luabinaries.luaforge.net/manual.html%23LuaBinariesCompatible&hl=en&ct=clnk&cd=1&gl=us&client=opera
> is it the same bug [they mention incompats when passing CRT objects among
> libraries]? dunno.

No, that page is concerned with multiple CRTs in the same process and passing around values between them (such as using fopen() in one CRT and fclose() in another).

This bug is about calling conventions for functions returning structs by value.

anybody know if this bug is related to this post? http://www.gamedev.net/community/forums/topic.asp?topic_id=482230
Thanks!
-=roger

The proposed patch works for plain C code, but also affects C++. Since libstdc++ contains code that returns aggregates or calls code that does, -mms-aggregate-return will make the generated code incompatible with the C++ library. This makes it impossible to write C++ code that links (as a DLL) to code compiled with Microsoft or Intel compilers, even if the interface is in plain C.

We are exactly in this situation. We will have to build libstdc++ with -mms-aggregate-return, but that option would then become mandatory for all C++ code. Things would be easier if the option were on by default - we can do this for our build locally, of course, but we'd rather not diverge too much from the standard behaviour.

An alternative would be to make the option only affect C (including extern "C" in C++), as other languages are not subject to compatibility with MS compilers.

(In reply to comment #7)
> The proposed patch works for plain C code, but also affects C++. Since
> libstdc++ contains code that returns aggregates or calls code that does,

An example, please.  The patch only affects functions that return large (> 8 bytes) aggregates by value.

Danny

> -mms-aggregate-return will make the generated code incompatible with the C++
> library. 

Created attachment 17508 [details]
Functions in libstdc++ returning aggregates > 8 bytes

These are the functions in libstdc++ that contain the instruction "ret 0x4", assuming that they correspond exactly to those returning aggregates larger than 8 bytes. There are 127 of them.

Note that C++ objects need not be larger than 8 bytes to qualify for returning on the stack (and thus subject to this cleanup problem). Any class with a copy constructor, for example, seems to be affected.
Try compiling:

struct A {
  A();
  A(const A&);
  int x;
};
A f() { return A(); }

This is probably why methods returning string::reverse_iterator make the list.
(http://www.agner.org/optimize/calling_conventions.pdf was useful for understanding this.)

(In reply to comment #10)
> Note that C++ objects need not be larger than 8 bytes to qualify for returning
> on the stack (and thus subject to this cleanup problem). Any class with a copy
> constructor, for example, seems to be affected.

Thanks.  I understand your concern now.  
Do you think that a function  __attibute__((ms-aggregate-return)) would be  useful to specify individual problematic functions.
I expect that this would be equivalent to  (__attribute__((target("ms-aggregate-return"))) which should work now, but I haven't tested,  Will do so soon.

As I read this. Would it make sense to use for x86-mingw the callabi feature (as we do for the x64 variant)? This would be useful for 32-bit based multilib version, too (but this is more a side-note for this).

Kai

Author: ktietz
Date: Sat Dec 18 10:16:13 2010
New Revision: 168019

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168019
Log:
2010-12-18  Kai Tietz  <kai.tietz@onevision.com>

	PR target/36834
	* config/i386/i386.c (ix86_keep_aggregate_return_pointer):
	New local function.
	(ix86_return_pops_args): Use ix86_keep_aggregate_return_pointer
	function instead of KEEP_AGGREGATE_RETURN_POINTER.
	(ix86_handle_callee_pop_aggregate_return): New handler.
	(ix86_attribute_table): Add new attribute
	callee_pop_aggregate_return.
	* doc/extend.texi (callee_pop_aggregate_return): Add
	attribute documentation.

2010-12-18  Kai Tietz  <kai.tietz@onevision.com>

	PR target/36834
	* gcc.target/i386/aggregate-ret1.c: New.
	* gcc.target/i386/aggregate-ret2.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/i386/aggregate-ret1.c
    trunk/gcc/testsuite/gcc.target/i386/aggregate-ret2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog

By this new attribute functions can be explicit marked to use VC compatible aggregate return pointer handling.
I don't plan to backmerge this fix to 4.5.x tree, so I close this bug as fixed for 4.6 tree. For 4.7 there is planned to introduce for mingw 32-bit target the callabi MS_ABI/SYSV_ABI (as it is already present for 64-bit mingw target). This will then automatically switch on MS specific calling convention changes - eg ms-bitfield, caller-pops-aggregate-return, and co.