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

[Charles Davis]

On Jul 24, 2013, at 11:13 PM, Reid Kleckner wrote:

> Sounds good mostly, but I happen to be in the middle of untangling the mess of CC_Default != CC_C.
> 
> One thing that should probably work here is if I'm targeting a platform where sysv_abi is equivalent to the default cc, a sysv_abi function should be compatible with a normal function pointer.  Similarly when targeting Windows, an ms_abi function is probably compatible with a normal function pointer.  Please see what gcc does and add tests that show that we match it.
That's definitely true, and I should've known better ;). In fact, GCC will allow a sysv_abi function to be assigned to a plain function pointer on non-Windows, or an ms_abi function to be assigned to a plain function pointer on Windows.

Some other findings of mine:

- The ms_abi and sysv_abi attributes cannot be used together in the same declaration. But it is possible to declare a function with one, and then with the other later. This behavior doesn't make sense to me. Since they aren't compatible, we should always err in both cases.
- Either one can be combined with cdecl. Personally, I think that should only be possible with the one that is the default for that platform (since cdecl maps to CC_C in the AST, and thus, to ccc in the IR), but I'll defer to your judgment.
- Either one can also be combined with the regparm() attribute. Apparently, GCC ignores regparm and cdecl on x86-64, just as it ignores ms_abi and sysv_abi on i386 (see below). We seem to ignore regparm() on x86-64 as well.
> 
> Hm, now I worry if we have a bug in LLVM.  LLVM will replace calls with the wrong cc with unreachable, I think, and it may not know that x86_64_sysv_cc is compatible with ccc on non-Windows OSs.
Great. That's one corner case I forgot. I'll have to make a patch LLVM to handle this. Looks like I'll have to add Anton's target CC hook after all.
> 
> Also, what does ms_abi do on x86_32?
In older versions of GCC (up to 4.6.x), it's ignored. In fact, you'll get a -Wattributes warning telling you this. That's why I didn't handle it for i386.
>  Is it stdcall or cdecl?  Add a test for i686.
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).

Chip

> 
> 
> On Wed, Jul 24, 2013 at 10:50 PM, Charles Davis <cdavis5x at gmail.com> wrote:
> 
> On Jul 12, 2013, at 5:48 AM, Reid Kleckner wrote:
> 
>> On Tue, Jun 18, 2013 at 6:25 AM, Benno Rice <benno at freebsd.org> wrote:
>> 
>> On 12/06/2013, at 10:40 PM, Reid Kleckner <rnk at google.com> wrote:
>> 
>>> On Wed, Jun 12, 2013 at 7:13 AM, Charles Davis <cdavis5x at gmail.com> wrote:
>>> 
>>> You mean like in my patch here:
>>> 
>>> http://llvm-reviews.chandlerc.com/D729
>>> 
>>> ?
>>> 
>>> Yeah, that looks good to me.  :)
>>> 
>>> From the docs, it seems attr(ms_abi) just maps to x86_64_win64cc and x86_stdcallcc depending on the architecture:
>>> http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html#index-g_t_0040code_007bsysv_005fabi_007d-attribute-2670
>>> gcc also appears to support a sysv_abi attribute, which lets you use the sysv CCs when targeting Windows.
>> 
>> So what's in the way of this being committed? Is there anything I can do that would accelerate things?
>> 
>> Benno, Charles added explicit sysv and win64 CCs to LLVM.  Feel free to update your clang patch based on that and resend. 
> I can wait no longer...
> 
> I've taken the Clang side of Benno's patch, updated it against my LLVM patch, and rebased it against trunk. I also added the complementary sysv_abi attribute.
> 
> OK to commit?
> 
> Chip
> 
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130725/5f887c4d/attachment.html>