RE: D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

[Erik Schwiebert via cfe-commits]

These hypothetical targets aren’t so hypothetical.  Office for Mac and iOS uses 
code ported from Windows but actually targeted at running on the Apple (Darwin) 
platform. We don’t have the MSVC CRT there.  

Schwieb

-Original Message-
From: Reid Kleckner via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Thursday, July 13, 2017 8:31 AM
To: bruno.card...@gmail.com; e...@efcs.ca; mclow.li...@gmail.com; 
dexonsm...@apple.com; ben.cr...@gmail.com; smee...@fb.com; 
compn...@compnerd.org; david.majne...@gmail.com
Cc: geek4ci...@gmail.com; Erik Schwiebert ; 
cfe-commits@lists.llvm.org
Subject: [PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

> Some non-windows targets using MS extensions define _WIN32 for compatibility 
> with Windows but do not have MSVC compatibility.

So, these hypothetical targets have the Win32 API available, but they do not 
use the MSVC CRT, correct? They could use musl, or some custom C runtime, but 
they don't want libc++ to aim for MSVCRT compatibility.

---

In any case, checking _MSC_VER is the best thing I can think of for guessing if 
the MSVC CRT is going to be present, so this looks good.


https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD34588=02%7C01%7Ceriksc%40microsoft.com%7C55099ebc746b4a80da4408d4ca042b95%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636355566643019338=n7xbFct8h7QlnsjjZifZYX72qqpzSG%2FpUAZEKbRbVY0%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

[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

[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

[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

[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 
Cc: Saleem Abdulrasool ; Albert Gutowski 
; David Majnemer ; cfe-commits 
; Erik Schwiebert 
Subject: Re: r284060 - Implement MS _BitScan intrinsics


> 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

[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' ; 'Duncan P. N. Exon Smith' 

Cc: 'Albert Gutowski' ; 'Reid Kleckner' 
; 'David Majnemer' ; 'cfe-commits' 

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' >; 
Duncan P. N. Exon Smith >
Cc: Albert Gutowski >; Reid 
Kleckner >; David Majnemer 
>; cfe-commits 
>
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 >
Cc: Albert Gutowski >; Reid 
Kleckner >; David Majnemer 
>; cfe-commits 
>; Erik 
Schwiebert >
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 
> 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 
> > 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: 

RE: r284060 - Implement MS _BitScan intrinsics

[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' ; Duncan P. N. Exon Smith 

Cc: Albert Gutowski ; Reid Kleckner ; 
David Majnemer ; cfe-commits 

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 >
Cc: Albert Gutowski >; Reid 
Kleckner >; David Majnemer 
>; cfe-commits 
>; Erik 
Schwiebert >
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 
> 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 
> > 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
> ==
> --- 

RE: r284060 - Implement MS _BitScan intrinsics

[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 
Cc: Albert Gutowski ; Reid Kleckner ; 
David Majnemer ; cfe-commits 
; Erik Schwiebert 
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 
> 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 
> > 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,