r284060 - Implement MS _BitScan intrinsics

Fri Jun 16 17:08:08 PDT 2017

> On Jun 16, 2017, at 11:02, Reid Kleckner <rnk at google.com> wrote:
> 
> We should fix it.

Agreed.

> We just need a new character code in the builtin function prototype encoding. Currently there is no encoding for a portable int32_t that magically becomes "long" on 32-bit Windows. The closest thing we have is 'W', which is used by Neon intrinsics to select between "long" and "long long". It was added in r202004.

Yup, I saw the 'W' as well.  That's the same approach I was using in a patch yesterday morning (I picked lowercase L ('l'), but there may be a better choice).  Likely Bruno will follow up with a working patch soon (I hit some problems with tests and then got tied up).

> On Fri, Jun 16, 2017 at 10:28 AM, Erik Schwiebert <eriksc at microsoft.com <mailto:eriksc at microsoft.com>> wrote:
> We (Office developers for Apple platforms at Microsoft) would prefer to have the intrinsics work properly for LP64, as that will generate the most optimal and efficient code. That said, we understand that this may be odd to implement from the compiler's perspective and can work around it if the intrinsics are not supported for ms-extensions+LP64. At the end of the day we need the compiler to clearly adopt one of those two options, and not sit somewhere in the middle as it currently does. We don't turn on any flags other than -fms-extensions; we don’t attempt to define MSC_VER and we do conditionalize code based on __clang__ so the code is aware it is being compiled by clang vs MSVC.
> 
> So I think we'd prefer what Duncan and Apple are offering to do, but if the larger clang community thinks that is a bad idea and wants to explicitly disable the intrinsics, we could live with that.
> 
> Thanks,
> Schwieb
> 
> -----Original Message-----
> From: Erik Schwiebert
> Sent: Friday, June 16, 2017 8:49 AM
> To: 'Bruno Cardoso Lopes' <bruno.cardoso at gmail.com <mailto:bruno.cardoso at gmail.com>>; Brian Kelley <bkelley at microsoft.com <mailto:bkelley at microsoft.com>>; Tomasz Kukielka <tkukielk at microsoft.com <mailto:tkukielk at microsoft.com>>
> Cc: dexonsmith at apple.com <mailto:dexonsmith at apple.com>; Reid Kleckner <rnk at google.com <mailto:rnk at google.com>>; cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
> Subject: RE: r284060 - Implement MS _BitScan intrinsics
> 
> Adding Brian and Tomasz. I'm pretty sure we have the Windows SDK intrinsics headers. I'm not sure which method we'd prefer, so I'll walk down the hall and ask them. Tomasz is our header maestro (because we play crazy #include_next games so we can use both Windows and macOS SDKs!)
> 
> We'll get back to you ASAP.
> 
> Schwieb
> 
> -----Original Message-----
> From: Bruno Cardoso Lopes [mailto:bruno.cardoso at gmail.com <mailto:bruno.cardoso at gmail.com>]
> Sent: Thursday, June 15, 2017 4:41 PM
> To: Erik Schwiebert <eriksc at microsoft.com <mailto:eriksc at microsoft.com>>
> Cc: dexonsmith at apple.com <mailto:dexonsmith at apple.com>; Reid Kleckner <rnk at google.com <mailto:rnk at google.com>>; cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
> Subject: Re: r284060 - Implement MS _BitScan intrinsics
> 
> On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
> <bruno.cardoso at gmail.com <mailto:bruno.cardoso at gmail.com>> wrote:
> > On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> > <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> >> SGTM too. Regarding Duncan's last question -- I can't think of any such customer. :) If you all think the right thing for clang to do is to infer LLP64 behavior on LP64 (Darwin) + ms_extensions, then that is fine with me!
> 
> Thinking more about this; what if we mark such builtins as unsupported
> for "LP64 (Darwin) + ms_extensions" and then you provide the
> definitions via intrin.h (we can generate a compiler macro for this
> scenario and conditionalize the include_next as we do for _MSC_VER)?
> Do you use this header at all? Are there any other MS related flags
> that get passed to the compiler?
> 
> > SGTM as well!
> >
> >>
> >> Thanks all!
> >> Schwieb
> >>
> >> -----Original Message-----
> >> From: dexonsmith at apple.com <mailto:dexonsmith at apple.com> [mailto:dexonsmith at apple.com <mailto:dexonsmith at apple.com>]
> >> Sent: Monday, June 12, 2017 1:55 PM
> >> To: Reid Kleckner <rnk at google.com <mailto:rnk at google.com>>
> >> Cc: Saleem Abdulrasool <compnerd at compnerd.org <mailto:compnerd at compnerd.org>>; Albert Gutowski <agutowski at google.com <mailto:agutowski at google.com>>; David Majnemer <david.majnemer at gmail.com <mailto:david.majnemer at gmail.com>>; cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>; Erik Schwiebert <eriksc at microsoft.com <mailto:eriksc at microsoft.com>>
> >> Subject: Re: r284060 - Implement MS _BitScan intrinsics
> >>
> >>
> >>> On Jun 12, 2017, at 12:44, Reid Kleckner <rnk at google.com <mailto:rnk at google.com>> wrote:
> >>>
> >>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compnerd at compnerd.org <mailto:compnerd at compnerd.org>> wrote:
> >>>> I'm worried about changing this signature all the time.  I suspect that it will cause the following to be emitted for valid code:
> >>>>
> >>>> warning: incompatible pointer types passing 'unsigned long *' to parameter of type 'unsigned int *' [-Wincompatible-pointer-types]
> >>>>
> >>>> Switching the signature on LP64 sounds much better to me.
> >>>
> >>> Right, we have to do this. It needs to be `long` on Windows.
> >>
> >> SGTM.  We'll go that way.
> >
> > +1 here!
> >
> >>> On Jun 8, 2017, at 12:21, Erik Schwiebert <eriksc at microsoft.com <mailto:eriksc at microsoft.com>> wrote:
> >>>
> >>> It’s probably also better to not try to infer our weird desired behavior. It should probably be controlled by a specific driver directive, like “-fms-extensions-lp64-intrinsics” or something like that. Using a new directive means that nobody can accidentally get this behavior if they for some reason do want LLP64 behavior with Windows intrinsics.
> >>
> >> This seems overly complicated.  Is there a customer that:
> >> - is on LP64,
> >> - is using -fms-extensions,
> >> - is using these intrinsics, and
> >> - wants them to be 64-bit longs instead of 32-bit ints?
> >> Put another way: who would use these intrinsics on LP64 and *not* want to mimic LLP64?
> >>
> >> If everyone using the intrinsics on LP64 is going to have to specify -fms-extensions-lp64-intrinsics, then we should just imply it.
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> >> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=iWTF3jpX71tU%2B6aq%2BEpv8VD8IFfeDKMHvZd40%2FK64aE%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=iWTF3jpX71tU%2B6aq%2BEpv8VD8IFfeDKMHvZd40%2FK64aE%3D&reserved=0>
> >
> >
> >
> > --
> > Bruno Cardoso Lopes
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D&reserved=0>
> 
> 
> 
> --
> Bruno Cardoso Lopes
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc&data=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946&sdata=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D&reserved=0>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170616/8e110240/attachment-0001.html>