RE: r284060 - Implement MS _BitScan intrinsics

2017-06-20 Thread Erik Schwiebert via cfe-commits 
LGTM, assuming "howLong" == 0 means 32-bit int. 

-Original Message-
From: Bruno Cardoso Lopes [mailto:bruno.card...@gmail.com] 
Sent: Monday, June 19, 2017 6:45 PM
To: Duncan P. N. Exon Smith <dexonsm...@apple.com>
Cc: Reid Kleckner <r...@google.com>; Erik Schwiebert <eri...@microsoft.com>; 
Brian Kelley <bkel...@microsoft.com>; Tomasz Kukielka <tkuki...@microsoft.com>; 
cfe-commits <cfe-commits@lists.llvm.org>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

On Fri, Jun 16, 2017 at 5:08 PM, Duncan P. N. Exon Smith
<dexonsm...@apple.com> wrote:
>
> On Jun 16, 2017, at 11:02, Reid Kleckner <r...@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).

Right, here it is: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD34377=02%7C01%7Ceriksc%40microsoft.com%7C622ea4cd65f846cd1be708d4b77dfaa1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636335199097092827=abfnUWviZ01v%2B%2B43Q6vLvZoBk2QbawkzKNpayIgMJwY%3D=0

Let me know what you guys think!

>
> On Fri, Jun 16, 2017 at 10:28 AM, Erik Schwiebert <eri...@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.card...@gmail.com>; Brian Kelley
>> <bkel...@microsoft.com>; Tomasz Kukielka <tkuki...@microsoft.com>
>> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits
>> <cfe-commits@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.card...@gmail.com]
>> Sent: Thursday, June 15, 2017 4:41 PM
>> To: Erik Schwiebert <eri...@microsoft.com>
>> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits
>> <cfe-commits@lists.llvm.org>
>> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>>
>> On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
>> <bruno.card...@gmail.com> wrote:
>> > On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
>> > <cfe-commits@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 gen

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-19 Thread Bruno Cardoso Lopes via cfe-commits 
On Fri, Jun 16, 2017 at 5:08 PM, Duncan P. N. Exon Smith
<dexonsm...@apple.com> wrote:
>
> On Jun 16, 2017, at 11:02, Reid Kleckner <r...@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).

Right, here it is: https://reviews.llvm.org/D34377

Let me know what you guys think!

>
> On Fri, Jun 16, 2017 at 10:28 AM, Erik Schwiebert <eri...@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.card...@gmail.com>; Brian Kelley
>> <bkel...@microsoft.com>; Tomasz Kukielka <tkuki...@microsoft.com>
>> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits
>> <cfe-commits@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.card...@gmail.com]
>> Sent: Thursday, June 15, 2017 4:41 PM
>> To: Erik Schwiebert <eri...@microsoft.com>
>> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits
>> <cfe-commits@lists.llvm.org>
>> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>>
>> On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
>> <bruno.card...@gmail.com> wrote:
>> > On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
>> > <cfe-commits@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: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
>> >> Sent: Monday, June 12, 2017 1:55 PM
>> >> To: Reid Kleckner <r...@google.com>
>> >> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski
>> >> <agutow...@google.com>; David Majnemer <david.

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-16 Thread Duncan P. N. Exon Smith via cfe-commits 

> On Jun 16, 2017, at 11:02, Reid Kleckner <r...@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 <eri...@microsoft.com 
> <mailto:eri...@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.card...@gmail.com 
> <mailto:bruno.card...@gmail.com>>; Brian Kelley <bkel...@microsoft.com 
> <mailto:bkel...@microsoft.com>>; Tomasz Kukielka <tkuki...@microsoft.com 
> <mailto:tkuki...@microsoft.com>>
> Cc: dexonsm...@apple.com <mailto:dexonsm...@apple.com>; Reid Kleckner 
> <r...@google.com <mailto:r...@google.com>>; cfe-commits 
> <cfe-commits@lists.llvm.org <mailto:cfe-commits@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.card...@gmail.com 
> <mailto:bruno.card...@gmail.com>]
> Sent: Thursday, June 15, 2017 4:41 PM
> To: Erik Schwiebert <eri...@microsoft.com <mailto:eri...@microsoft.com>>
> Cc: dexonsm...@apple.com <mailto:dexonsm...@apple.com>; Reid Kleckner 
> <r...@google.com <mailto:r...@google.com>>; cfe-commits 
> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>>
> Subject: Re: r284060 - Implement MS _BitScan intrinsics
> 
> On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
> <bruno.card...@gmail.com <mailto:bruno.card...@gmail.com>> wrote:
> > On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> > <cfe-commits@lists.llvm.org <mailto:cfe-commits@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: dexonsm...@apple.com <mailto:dexonsm...@apple.com> 
> >> [mailto:dexonsm...@apple.com <mailto:dexonsm...@apple.com>]
> >> Sent: Monday, June 12, 2017 1:55 PM
> >> To: Reid Kleckner <r...@google.com <mailto:r...@google.com>>

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-16 Thread Reid Kleckner via cfe-commits 
We should fix it. 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.

On Fri, Jun 16, 2017 at 10:28 AM, Erik Schwiebert <eri...@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.card...@gmail.com>; Brian Kelley <
> bkel...@microsoft.com>; Tomasz Kukielka <tkuki...@microsoft.com>
> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits <
> cfe-commits@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.card...@gmail.com]
> Sent: Thursday, June 15, 2017 4:41 PM
> To: Erik Schwiebert <eri...@microsoft.com>
> Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits <
> cfe-commits@lists.llvm.org>
> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>
> On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
> <bruno.card...@gmail.com> wrote:
> > On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> > <cfe-commits@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: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
> >> Sent: Monday, June 12, 2017 1:55 PM
> >> To: Reid Kleckner <r...@google.com>
> >> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski <
> agutow...@google.com>; David Majnemer <david.majne...@gmail.com>;
> cfe-commits <cfe-commits@lists.llvm.org>; Erik Schwiebert <
> eri...@microsoft.com>
> >> Subject: Re: r284060 - Implement MS _BitScan intrinsics
> >>
> >>
> >>> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
> >>>
> >>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <
> compn...@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 Win

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-16 Thread Erik Schwiebert via cfe-commits 
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.card...@gmail.com>; Brian Kelley 
<bkel...@microsoft.com>; Tomasz Kukielka <tkuki...@microsoft.com>
Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits 
<cfe-commits@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.card...@gmail.com] 
Sent: Thursday, June 15, 2017 4:41 PM
To: Erik Schwiebert <eri...@microsoft.com>
Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits 
<cfe-commits@lists.llvm.org>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
<bruno.card...@gmail.com> wrote:
> On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> <cfe-commits@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: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
>> Sent: Monday, June 12, 2017 1:55 PM
>> To: Reid Kleckner <r...@google.com>
>> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski 
>> <agutow...@google.com>; David Majnemer <david.majne...@gmail.com>; 
>> cfe-commits <cfe-commits@lists.llvm.org>; Erik Schwiebert 
>> <eri...@microsoft.com>
>> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>>
>>
>>> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
>>>
>>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compn...@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 <eri...@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
>

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-16 Thread Erik Schwiebert via cfe-commits 
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.card...@gmail.com] 
Sent: Thursday, June 15, 2017 4:41 PM
To: Erik Schwiebert <eri...@microsoft.com>
Cc: dexonsm...@apple.com; Reid Kleckner <r...@google.com>; cfe-commits 
<cfe-commits@lists.llvm.org>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
<bruno.card...@gmail.com> wrote:
> On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> <cfe-commits@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: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
>> Sent: Monday, June 12, 2017 1:55 PM
>> To: Reid Kleckner <r...@google.com>
>> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski 
>> <agutow...@google.com>; David Majnemer <david.majne...@gmail.com>; 
>> cfe-commits <cfe-commits@lists.llvm.org>; Erik Schwiebert 
>> <eri...@microsoft.com>
>> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>>
>>
>>> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
>>>
>>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compn...@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 <eri...@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@lists.llvm.org
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946=iWTF3jpX71tU%2B6aq%2BEpv8VD8IFfeDKMHvZd40%2FK64aE%3D=0
>
>
>
> --
> Bruno Cardoso Lopes
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D=0



-- 
Bruno Cardoso Lopes
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.brunocardoso.cc=02%7C01%7Ceriksc%40microsoft.com%7Cba4835eb4a1b438793d508d4b4481355%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636331669036098946=O01DYn4W3JN54%2B6wwAgCLAkq63PUtUBy4mQ9RMN833s%3D=0
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-15 Thread Bruno Cardoso Lopes via cfe-commits 
On Tue, Jun 13, 2017 at 8:13 PM, Bruno Cardoso Lopes
<bruno.card...@gmail.com> wrote:
> On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
> <cfe-commits@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: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
>> Sent: Monday, June 12, 2017 1:55 PM
>> To: Reid Kleckner <r...@google.com>
>> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski 
>> <agutow...@google.com>; David Majnemer <david.majne...@gmail.com>; 
>> cfe-commits <cfe-commits@lists.llvm.org>; Erik Schwiebert 
>> <eri...@microsoft.com>
>> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>>
>>
>>> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
>>>
>>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compn...@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 <eri...@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@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-13 Thread Bruno Cardoso Lopes via cfe-commits 
On Mon, Jun 12, 2017 at 2:01 PM, Erik Schwiebert via cfe-commits
<cfe-commits@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!

SGTM as well!

>
> Thanks all!
> Schwieb
>
> -Original Message-
> From: dexonsm...@apple.com [mailto:dexonsm...@apple.com]
> Sent: Monday, June 12, 2017 1:55 PM
> To: Reid Kleckner <r...@google.com>
> Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski 
> <agutow...@google.com>; David Majnemer <david.majne...@gmail.com>; 
> cfe-commits <cfe-commits@lists.llvm.org>; Erik Schwiebert 
> <eri...@microsoft.com>
> Subject: Re: r284060 - Implement MS _BitScan intrinsics
>
>
>> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
>>
>>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compn...@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 <eri...@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@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-12 Thread Erik Schwiebert via cfe-commits 
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!

Thanks all!
Schwieb

-Original Message-
From: dexonsm...@apple.com [mailto:dexonsm...@apple.com] 
Sent: Monday, June 12, 2017 1:55 PM
To: Reid Kleckner <r...@google.com>
Cc: Saleem Abdulrasool <compn...@compnerd.org>; Albert Gutowski 
<agutow...@google.com>; David Majnemer <david.majne...@gmail.com>; cfe-commits 
<cfe-commits@lists.llvm.org>; Erik Schwiebert <eri...@microsoft.com>
Subject: Re: r284060 - Implement MS _BitScan intrinsics


> On Jun 12, 2017, at 12:44, Reid Kleckner <r...@google.com> wrote:
> 
>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool <compn...@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.

> On Jun 8, 2017, at 12:21, Erik Schwiebert <eri...@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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-12 Thread Duncan P. N. Exon Smith via cfe-commits 

> On Jun 12, 2017, at 12:44, Reid Kleckner  wrote:
> 
>> On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool  
>> 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.

> On Jun 8, 2017, at 12:21, Erik Schwiebert  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@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-12 Thread Reid Kleckner via cfe-commits 
On Wed, Jun 7, 2017 at 7:31 PM, Saleem Abdulrasool 
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.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-12 Thread Erik Schwiebert via cfe-commits 
Any more thoughts on this from Saleem or Apple folks?

Thanks,
Schwieb

From: Erik Schwiebert
Sent: Thursday, June 8, 2017 12:22 PM
To: 'Saleem Abdulrasool' <compn...@compnerd.org>; 'Duncan P. N. Exon Smith' 
<dexonsm...@apple.com>
Cc: 'Albert Gutowski' <agutow...@google.com>; 'Reid Kleckner' 
<r...@google.com>; 'David Majnemer' <david.majne...@gmail.com>; 'cfe-commits' 
<cfe-commits@lists.llvm.org>
Subject: RE: r284060 - Implement MS _BitScan intrinsics

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.

Schwieb

From: Erik Schwiebert
Sent: Thursday, June 8, 2017 10:29 AM
To: 'Saleem Abdulrasool' <compn...@compnerd.org<mailto:compn...@compnerd.org>>; 
Duncan P. N. Exon Smith <dexonsm...@apple.com<mailto:dexonsm...@apple.com>>
Cc: Albert Gutowski <agutow...@google.com<mailto:agutow...@google.com>>; Reid 
Kleckner <r...@google.com<mailto:r...@google.com>>; David Majnemer 
<david.majne...@gmail.com<mailto:david.majne...@gmail.com>>; cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>
Subject: RE: r284060 - Implement MS _BitScan intrinsics

Yes, we definitely do not want to introduce pointer-type incompatibility 
warnings. Predicating the behavior change on LP64 vs LLP64 seems better.

What’s the best way to do that?  Brian and I can hack on clang a bit, but we’re 
certainly not experts.

Schwieb

From: Saleem Abdulrasool [mailto:compn...@compnerd.org]
Sent: Wednesday, June 7, 2017 7:32 PM
To: Duncan P. N. Exon Smith <dexonsm...@apple.com<mailto:dexonsm...@apple.com>>
Cc: Albert Gutowski <agutow...@google.com<mailto:agutow...@google.com>>; Reid 
Kleckner <r...@google.com<mailto:r...@google.com>>; David Majnemer 
<david.majne...@gmail.com<mailto:david.majne...@gmail.com>>; cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>; Erik 
Schwiebert <eri...@microsoft.com<mailto:eri...@microsoft.com>>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

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.

On Wed, Jun 7, 2017 at 2:56 PM, Duncan P. N. Exon Smith via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
[... excuse the necromancy...]

Hi Albert (and Reid and David),

This commit is breaking some uses of -fms-extensions on Apple platforms.  In 
particular, Brian and Erik (CC'ed) build against a version of the Windows SDK 
on Apple platforms.  _BitScanReverse is expected to be 32-bit, matching 
Windows/LLP64, even though long is 64-bit on Darwin/LP64.

One idea we've had for fixing this is to use "int" instead of "long" for these 
intrinsics, either:
- all the time, or
- when in LP64 mode (e.g., Darwin + -fms-extensions).

Any other ideas?

Thanks,
Duncan

> On Oct 12, 2016, at 15:01, Albert Gutowski via cfe-commits 
> <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
>
> Author: agutowski
> Date: Wed Oct 12 17:01:05 2016
> New Revision: 284060
>
> URL: 
> http://llvm.org/viewvc/llvm-project?rev=284060=rev<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D284060%26view%3Drev=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=uIMEYcgjFKByhB81x%2FX7%2B0Wm3HMK3hURS4yN7AsKbbI%3D=0>
> Log:
> Implement MS _BitScan intrinsics
>
> Summary: _BitScan intrinsics (and some others, for example _Interlocked and 
> _bittest) are supposed to work on both ARM and x86. This is an attempt to 
> isolate them, avoiding repeating their code or writing separate function for 
> each builtin.
>
> Reviewers: hans, thakis, rnk, majnemer
>
> Subscribers: RKSimon, cfe-commits, aemerson
>
> Differential Revision: 
> https://reviews.llvm.org/D25264<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD25264=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=Gwet6%2FKOIUgc03VtXdmk%2BTah9VXUrLReutN36lw3caw%3D=0>
>
> Modified:
>cfe/trunk/include/clang/Basic/BuiltinsARM.def
>cfe/trunk/include/clang/Basic

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-08 Thread Erik Schwiebert via cfe-commits 
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.

Schwieb

From: Erik Schwiebert
Sent: Thursday, June 8, 2017 10:29 AM
To: 'Saleem Abdulrasool' <compn...@compnerd.org>; Duncan P. N. Exon Smith 
<dexonsm...@apple.com>
Cc: Albert Gutowski <agutow...@google.com>; Reid Kleckner <r...@google.com>; 
David Majnemer <david.majne...@gmail.com>; cfe-commits 
<cfe-commits@lists.llvm.org>
Subject: RE: r284060 - Implement MS _BitScan intrinsics

Yes, we definitely do not want to introduce pointer-type incompatibility 
warnings. Predicating the behavior change on LP64 vs LLP64 seems better.

What’s the best way to do that?  Brian and I can hack on clang a bit, but we’re 
certainly not experts.

Schwieb

From: Saleem Abdulrasool [mailto:compn...@compnerd.org]
Sent: Wednesday, June 7, 2017 7:32 PM
To: Duncan P. N. Exon Smith <dexonsm...@apple.com<mailto:dexonsm...@apple.com>>
Cc: Albert Gutowski <agutow...@google.com<mailto:agutow...@google.com>>; Reid 
Kleckner <r...@google.com<mailto:r...@google.com>>; David Majnemer 
<david.majne...@gmail.com<mailto:david.majne...@gmail.com>>; cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>; Erik 
Schwiebert <eri...@microsoft.com<mailto:eri...@microsoft.com>>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

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.

On Wed, Jun 7, 2017 at 2:56 PM, Duncan P. N. Exon Smith via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
[... excuse the necromancy...]

Hi Albert (and Reid and David),

This commit is breaking some uses of -fms-extensions on Apple platforms.  In 
particular, Brian and Erik (CC'ed) build against a version of the Windows SDK 
on Apple platforms.  _BitScanReverse is expected to be 32-bit, matching 
Windows/LLP64, even though long is 64-bit on Darwin/LP64.

One idea we've had for fixing this is to use "int" instead of "long" for these 
intrinsics, either:
- all the time, or
- when in LP64 mode (e.g., Darwin + -fms-extensions).

Any other ideas?

Thanks,
Duncan

> On Oct 12, 2016, at 15:01, Albert Gutowski via cfe-commits 
> <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
>
> Author: agutowski
> Date: Wed Oct 12 17:01:05 2016
> New Revision: 284060
>
> URL: 
> http://llvm.org/viewvc/llvm-project?rev=284060=rev<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D284060%26view%3Drev=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=uIMEYcgjFKByhB81x%2FX7%2B0Wm3HMK3hURS4yN7AsKbbI%3D=0>
> Log:
> Implement MS _BitScan intrinsics
>
> Summary: _BitScan intrinsics (and some others, for example _Interlocked and 
> _bittest) are supposed to work on both ARM and x86. This is an attempt to 
> isolate them, avoiding repeating their code or writing separate function for 
> each builtin.
>
> Reviewers: hans, thakis, rnk, majnemer
>
> Subscribers: RKSimon, cfe-commits, aemerson
>
> Differential Revision: 
> https://reviews.llvm.org/D25264<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD25264=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=Gwet6%2FKOIUgc03VtXdmk%2BTah9VXUrLReutN36lw3caw%3D=0>
>
> Modified:
>cfe/trunk/include/clang/Basic/BuiltinsARM.def
>cfe/trunk/include/clang/Basic/BuiltinsX86.def
>cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
>cfe/trunk/lib/Basic/Targets.cpp
>cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>cfe/trunk/lib/CodeGen/CodeGenFunction.h
>cfe/trunk/lib/Headers/intrin.h
>cfe/trunk/test/CodeGen/ms-intrinsics.c
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=284060=284059=284060=diff<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Finclude%2Fclang%2FBasic%2FBuiltinsARM.def%3Frev%3D284060%26r1%3D284059%26r2%3D284060%26view%3Ddiff=02%7C01%7Ceriksc%40m

RE: r284060 - Implement MS _BitScan intrinsics

2017-06-08 Thread Erik Schwiebert via cfe-commits 
Yes, we definitely do not want to introduce pointer-type incompatibility 
warnings. Predicating the behavior change on LP64 vs LLP64 seems better.

What’s the best way to do that?  Brian and I can hack on clang a bit, but we’re 
certainly not experts.

Schwieb

From: Saleem Abdulrasool [mailto:compn...@compnerd.org]
Sent: Wednesday, June 7, 2017 7:32 PM
To: Duncan P. N. Exon Smith <dexonsm...@apple.com>
Cc: Albert Gutowski <agutow...@google.com>; Reid Kleckner <r...@google.com>; 
David Majnemer <david.majne...@gmail.com>; cfe-commits 
<cfe-commits@lists.llvm.org>; Erik Schwiebert <eri...@microsoft.com>
Subject: Re: r284060 - Implement MS _BitScan intrinsics

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.

On Wed, Jun 7, 2017 at 2:56 PM, Duncan P. N. Exon Smith via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
[... excuse the necromancy...]

Hi Albert (and Reid and David),

This commit is breaking some uses of -fms-extensions on Apple platforms.  In 
particular, Brian and Erik (CC'ed) build against a version of the Windows SDK 
on Apple platforms.  _BitScanReverse is expected to be 32-bit, matching 
Windows/LLP64, even though long is 64-bit on Darwin/LP64.

One idea we've had for fixing this is to use "int" instead of "long" for these 
intrinsics, either:
- all the time, or
- when in LP64 mode (e.g., Darwin + -fms-extensions).

Any other ideas?

Thanks,
Duncan

> On Oct 12, 2016, at 15:01, Albert Gutowski via cfe-commits 
> <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
>
> Author: agutowski
> Date: Wed Oct 12 17:01:05 2016
> New Revision: 284060
>
> URL: 
> http://llvm.org/viewvc/llvm-project?rev=284060=rev<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D284060%26view%3Drev=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=uIMEYcgjFKByhB81x%2FX7%2B0Wm3HMK3hURS4yN7AsKbbI%3D=0>
> Log:
> Implement MS _BitScan intrinsics
>
> Summary: _BitScan intrinsics (and some others, for example _Interlocked and 
> _bittest) are supposed to work on both ARM and x86. This is an attempt to 
> isolate them, avoiding repeating their code or writing separate function for 
> each builtin.
>
> Reviewers: hans, thakis, rnk, majnemer
>
> Subscribers: RKSimon, cfe-commits, aemerson
>
> Differential Revision: 
> https://reviews.llvm.org/D25264<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD25264=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=Gwet6%2FKOIUgc03VtXdmk%2BTah9VXUrLReutN36lw3caw%3D=0>
>
> Modified:
>cfe/trunk/include/clang/Basic/BuiltinsARM.def
>cfe/trunk/include/clang/Basic/BuiltinsX86.def
>cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
>cfe/trunk/lib/Basic/Targets.cpp
>cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>cfe/trunk/lib/CodeGen/CodeGenFunction.h
>cfe/trunk/lib/Headers/intrin.h
>cfe/trunk/test/CodeGen/ms-intrinsics.c
>
> Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=284060=284059=284060=diff<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Finclude%2Fclang%2FBasic%2FBuiltinsARM.def%3Frev%3D284060%26r1%3D284059%26r2%3D284060%26view%3Ddiff=02%7C01%7Ceriksc%40microsoft.com%7C2a2522c4af134940d62708d4ae16832e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636324859101456322=cse0wyDrNGXlbDkfKXjTS3NKWVc01xiwMFfvoBCzAXw%3D=0>
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Wed Oct 12 17:01:05 2016
> @@ -18,6 +18,10 @@
> #   define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS)
> #endif
>
> +#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
> +#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
> BUILTIN(ID, TYPE, ATTRS)
> +#endif
> +
> // In libgcc
> BUILTIN(__clear_cache, "vv*v*", "i")
>
> @@ -129,5 +133,11 @@ LANGBUILTIN(_MoveFromCoprocessor2, "UiIU
> LANGBUILTIN(_MoveToCoprocessor, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES)
> LANGBUILTIN(

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-07 Thread Saleem Abdulrasool via cfe-commits 
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.

On Wed, Jun 7, 2017 at 2:56 PM, Duncan P. N. Exon Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> [... excuse the necromancy...]
>
> Hi Albert (and Reid and David),
>
> This commit is breaking some uses of -fms-extensions on Apple platforms.
> In particular, Brian and Erik (CC'ed) build against a version of the
> Windows SDK on Apple platforms.  _BitScanReverse is expected to be 32-bit,
> matching Windows/LLP64, even though long is 64-bit on Darwin/LP64.
>
> One idea we've had for fixing this is to use "int" instead of "long" for
> these intrinsics, either:
> - all the time, or
> - when in LP64 mode (e.g., Darwin + -fms-extensions).
>
> Any other ideas?
>
> Thanks,
> Duncan
>
> > On Oct 12, 2016, at 15:01, Albert Gutowski via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: agutowski
> > Date: Wed Oct 12 17:01:05 2016
> > New Revision: 284060
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=284060=rev
> > Log:
> > Implement MS _BitScan intrinsics
> >
> > Summary: _BitScan intrinsics (and some others, for example _Interlocked
> and _bittest) are supposed to work on both ARM and x86. This is an attempt
> to isolate them, avoiding repeating their code or writing separate function
> for each builtin.
> >
> > Reviewers: hans, thakis, rnk, majnemer
> >
> > Subscribers: RKSimon, cfe-commits, aemerson
> >
> > Differential Revision: https://reviews.llvm.org/D25264
> >
> > Modified:
> >cfe/trunk/include/clang/Basic/BuiltinsARM.def
> >cfe/trunk/include/clang/Basic/BuiltinsX86.def
> >cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
> >cfe/trunk/lib/Basic/Targets.cpp
> >cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> >cfe/trunk/lib/CodeGen/CodeGenFunction.h
> >cfe/trunk/lib/Headers/intrin.h
> >cfe/trunk/test/CodeGen/ms-intrinsics.c
> >
> > Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/BuiltinsARM.def?rev=284060=284059=284060=diff
> > 
> ==
> > --- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original)
> > +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Wed Oct 12 17:01:05
> 2016
> > @@ -18,6 +18,10 @@
> > #   define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE,
> ATTRS)
> > #endif
> >
> > +#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
> > +#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE)
> BUILTIN(ID, TYPE, ATTRS)
> > +#endif
> > +
> > // In libgcc
> > BUILTIN(__clear_cache, "vv*v*", "i")
> >
> > @@ -129,5 +133,11 @@ LANGBUILTIN(_MoveFromCoprocessor2, "UiIU
> > LANGBUILTIN(_MoveToCoprocessor, "vUiIUiIUiIUiIUiIUi", "",
> ALL_MS_LANGUAGES)
> > LANGBUILTIN(_MoveToCoprocessor2, "vUiIUiIUiIUiIUiIUi", "",
> ALL_MS_LANGUAGES)
> >
> > +TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > +TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "n",
> "intrin.h", ALL_MS_LANGUAGES, "")
> > +TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "n",
> "intrin.h", ALL_MS_LANGUAGES, "")
> > +
> > #undef BUILTIN
> > #undef LANGBUILTIN
> > +#undef TARGET_HEADER_BUILTIN
> >
> > Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/BuiltinsX86.def?rev=284060=284059=284060=diff
> > 
> ==
> > --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> > +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Oct 12 17:01:05
> 2016
> > @@ -2028,6 +2028,10 @@ TARGET_BUILTIN(__builtin_ia32_selectpd_5
> > TARGET_BUILTIN(__builtin_ia32_monitorx, "vv*UiUi", "", "mwaitx")
> > TARGET_BUILTIN(__builtin_ia32_mwaitx, "vUiUiUi", "", "mwaitx")
> >
> > +// MSVC
> > +TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > +
> > TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h",
> ALL_MS_LANGUAGES, "")
> > TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", "intrin.h",
> ALL_MS_LANGUAGES, "")
> >
> > Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/BuiltinsX86_64.def?rev=284060=284059=284060=diff
> >

Re: r284060 - Implement MS _BitScan intrinsics

2017-06-07 Thread Duncan P. N. Exon Smith via cfe-commits 
[... excuse the necromancy...]

Hi Albert (and Reid and David),

This commit is breaking some uses of -fms-extensions on Apple platforms.  In 
particular, Brian and Erik (CC'ed) build against a version of the Windows SDK 
on Apple platforms.  _BitScanReverse is expected to be 32-bit, matching 
Windows/LLP64, even though long is 64-bit on Darwin/LP64.

One idea we've had for fixing this is to use "int" instead of "long" for these 
intrinsics, either:
- all the time, or
- when in LP64 mode (e.g., Darwin + -fms-extensions).

Any other ideas?

Thanks,
Duncan

> On Oct 12, 2016, at 15:01, Albert Gutowski via cfe-commits 
>  wrote:
> 
> Author: agutowski
> Date: Wed Oct 12 17:01:05 2016
> New Revision: 284060
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=284060=rev
> Log:
> Implement MS _BitScan intrinsics
> 
> Summary: _BitScan intrinsics (and some others, for example _Interlocked and 
> _bittest) are supposed to work on both ARM and x86. This is an attempt to 
> isolate them, avoiding repeating their code or writing separate function for 
> each builtin.
> 
> Reviewers: hans, thakis, rnk, majnemer
> 
> Subscribers: RKSimon, cfe-commits, aemerson
> 
> Differential Revision: https://reviews.llvm.org/D25264
> 
> Modified:
>cfe/trunk/include/clang/Basic/BuiltinsARM.def
>cfe/trunk/include/clang/Basic/BuiltinsX86.def
>cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
>cfe/trunk/lib/Basic/Targets.cpp
>cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>cfe/trunk/lib/CodeGen/CodeGenFunction.h
>cfe/trunk/lib/Headers/intrin.h
>cfe/trunk/test/CodeGen/ms-intrinsics.c
> 
> Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=284060=284059=284060=diff
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Wed Oct 12 17:01:05 2016
> @@ -18,6 +18,10 @@
> #   define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS)
> #endif
> 
> +#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
> +#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
> BUILTIN(ID, TYPE, ATTRS)
> +#endif
> +
> // In libgcc
> BUILTIN(__clear_cache, "vv*v*", "i")
> 
> @@ -129,5 +133,11 @@ LANGBUILTIN(_MoveFromCoprocessor2, "UiIU
> LANGBUILTIN(_MoveToCoprocessor, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES)
> LANGBUILTIN(_MoveToCoprocessor2, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES)
> 
> +TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +
> #undef BUILTIN
> #undef LANGBUILTIN
> +#undef TARGET_HEADER_BUILTIN
> 
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=284060=284059=284060=diff
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Oct 12 17:01:05 2016
> @@ -2028,6 +2028,10 @@ TARGET_BUILTIN(__builtin_ia32_selectpd_5
> TARGET_BUILTIN(__builtin_ia32_monitorx, "vv*UiUi", "", "mwaitx")
> TARGET_BUILTIN(__builtin_ia32_mwaitx, "vUiUiUi", "", "mwaitx")
> 
> +// MSVC
> +TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +
> TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> 
> Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=284060=284059=284060=diff
> ==
> --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
> +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Wed Oct 12 17:01:05 2016
> @@ -22,6 +22,9 @@
> #  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
> BUILTIN(ID, TYPE, ATTRS)
> #endif
> 
> +TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "n", "intrin.h", 
> ALL_MS_LANGUAGES, "")
> +
> TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi","nch", "intrin.h", 
> ALL_MS_LANGUAGES,

r284060 - Implement MS _BitScan intrinsics

2016-10-12 Thread Albert Gutowski via cfe-commits 
Author: agutowski
Date: Wed Oct 12 17:01:05 2016
New Revision: 284060

URL: http://llvm.org/viewvc/llvm-project?rev=284060=rev
Log:
Implement MS _BitScan intrinsics

Summary: _BitScan intrinsics (and some others, for example _Interlocked and 
_bittest) are supposed to work on both ARM and x86. This is an attempt to 
isolate them, avoiding repeating their code or writing separate function for 
each builtin.

Reviewers: hans, thakis, rnk, majnemer

Subscribers: RKSimon, cfe-commits, aemerson

Differential Revision: https://reviews.llvm.org/D25264

Modified:
cfe/trunk/include/clang/Basic/BuiltinsARM.def
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=284060=284059=284060=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Wed Oct 12 17:01:05 2016
@@ -18,6 +18,10 @@
 #   define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS)
 #endif
 
+#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
+#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
+#endif
+
 // In libgcc
 BUILTIN(__clear_cache, "vv*v*", "i")
 
@@ -129,5 +133,11 @@ LANGBUILTIN(_MoveFromCoprocessor2, "UiIU
 LANGBUILTIN(_MoveToCoprocessor, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES)
 LANGBUILTIN(_MoveToCoprocessor2, "vUiIUiIUiIUiIUiIUi", "", ALL_MS_LANGUAGES)
 
+TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+
 #undef BUILTIN
 #undef LANGBUILTIN
+#undef TARGET_HEADER_BUILTIN

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=284060=284059=284060=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Oct 12 17:01:05 2016
@@ -2028,6 +2028,10 @@ TARGET_BUILTIN(__builtin_ia32_selectpd_5
 TARGET_BUILTIN(__builtin_ia32_monitorx, "vv*UiUi", "", "mwaitx")
 TARGET_BUILTIN(__builtin_ia32_mwaitx, "vUiUiUi", "", "mwaitx")
 
+// MSVC
+TARGET_HEADER_BUILTIN(_BitScanForward, "UcULi*ULi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_BitScanReverse, "UcULi*ULi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=284060=284059=284060=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Wed Oct 12 17:01:05 2016
@@ -22,6 +22,9 @@
 #  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
 #endif
 
+TARGET_HEADER_BUILTIN(_BitScanForward64, "UcULi*ULLi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcULi*ULLi", "n", "intrin.h", 
ALL_MS_LANGUAGES, "")
+
 TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi","nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLLiLLi*",  "nh",   "intrin.h", 
ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/lib/Basic/Targets.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=284060=284059=284060=diff
==
--- cfe/trunk/lib/Basic/Targets.cpp (original)
+++ cfe/trunk/lib/Basic/Targets.cpp Wed Oct 12 17:01:05 2016
@@ -5588,6 +5588,8 @@ const Builtin::Info ARMTargetInfo::Built
   { #ID, TYPE, ATTRS, nullptr, LANG, nullptr },
 #define LIBBUILTIN(ID, TYPE, ATTRS, HEADER) \
   { #ID, TYPE, ATTRS, HEADER, ALL_LANGUAGES, nullptr },
+#define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANGS, FEATURE) \
+  { #ID, TYPE, ATTRS, HEADER,