Re: [PATCH] Bug 14984 - Implement __attribute__((ms_abi))

Thu, 29 Aug 2013 15:14:17 -0700


On Aug 29, 2013, at 11:32 AM, Reid Kleckner wrote:

I think the lesson of CC_Default vs. CC_C is that there are *far* too many places to check for calling convention compatibility, and that it's much better to use a single representation for equivalent conventions.

Therefore, I'd remove areCCsCompatible() and let TargetInfo::checkCallingConvention() adjust the the sysv or MS CCs to CC_C when appropriate.

This way, when you target x64 Windows, the ms_abi attr will show up in the AST, but it will produce function types that are nicely canonically equivalent to normal function types.  Similarly, sysv_abi will be a no-op on sysv targets.
You mean, like this?

There's one side effect of your suggested change that still bugs me, though. Now, if we're on a target that uses the SysV calling convention, the diagnostics always say "cdecl", even if the function was explicitly declared "sysv_abi". (Same with Windows and "ms_abi".) This might be slightly confusing to your average user, but for obvious reasons I don't want to add special cases to the places these diagnostics get printed.

Chip

Attachment: 0001-Add-ms_abi-and-sysv_abi-attribute-handling.patch
Description: Binary data




On Wed, Aug 28, 2013 at 9:40 PM, Charles Davis <[email protected]> wrote:

On Aug 19, 2013, at 3:15 PM, Charles Davis wrote:


On Aug 19, 2013, at 2:06 PM, Reid Kleckner wrote:

On Thu, Jul 25, 2013 at 7:27 PM, Charles Davis <[email protected]> wrote:
Actually, it's whatever the default calling convention is. Normally, that's __cdecl, unless you pass -mrtd; then it's __stdcall. It's still ignored on i386; it's just that gcc won't warn on it anymore (for some reason). I haven't been able to find anything in their Bugzilla about it. (This is at least true on 4.7, but I really don't feel like building 4.8 to find out if that's changed. It took all day just for the computer running Linux to build 4.7 so I could verify this for you. ;)

The question is, since it has no effect on any platform except x86-64, should *we* warn about it? I think we should.

I have a new patch, but I need to test it first. I'm also tired. I'll have it for you tomorrow morning (hopefully).

 Any news on this front?
Yeah, but it's bad news.

I haven't yet been able to figure out how to make Clang think cdecl is compatible with whatever calling convention (ms_abi or sysv_abi) is the default without breaking a bunch of other stuff. I was waiting for your CC_Default patch to go in, because I was hoping that would make it easier.

Maybe it would just be easier for now to do what GCC does, and just declare that cdecl is compatible with both no matter which target we're targeting.

Since I've had no luck working on this, I've had this set aside for now. I'll attach what I've got for now, but (since you're still busy with the CC_Default patch) I don't imagine you'll have a lot of time to work on this. I'll see if I can make this work in the meantime.
Now that your CC_Default patch is in, I've had better luck.

This patch passes all the tests I added. PTAL.

Chip




_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to