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

[Charles Davis]

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

> 
> 
> On Wed, Aug 28, 2013 at 9:40 PM, Charles Davis <cdavis5x at gmail.com> 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 <cdavis5x at gmail.com> 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
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/7821b8f0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-ms_abi-and-sysv_abi-attribute-handling.patch
Type: application/octet-stream
Size: 17467 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/7821b8f0/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130829/7821b8f0/attachment-0001.html>