Aha, which version of clang-cl are you using? With the released version of clang-cl 9.0.1, I get a warning instead of an error:

Eh, no it does not crash clang, at least not here? Instead it gives you a compile error, as it should:

LGTM.

We should abandon this, for e.g. 9.0.1 the whole Base_url variable has become unused. :)

In D69990#1750348, @tra wrote:

LGTM, though I'm curious if it's particularly useful. Last time I checked NVIDIA didn't ship libcudart for FreeBSD and without it it's rather cumbersome to use CUDA in practice.

Adding a few people who might know a bit more about CUDA specific things. Please take a look if this review makes any sense, thanks. :)

• Add cuda options test, copied from cuda-options.cu.

I'll work on a test.

Now opt supports -disable-builtin, move the test to llvm/test/Transforms/InstCombine.

In D56398#1744265, @ldionne wrote:

Sorry if this is a basic question, but why are we trying to provide EINTEGRITY in libc++'s errno.h header at all? I don't see this being part of the C or C++ Standard.

I submitted D70193 for adding a -disable-builtin option to opt. Once that is committed, this review can continue.

In D56398#1742899, @xbolva00 wrote:

cem == dim?

In D70143#1742885, @lebedev.ri wrote:

This should have a llvm ir test in llvm/test/transforms/instcombine i think, not a clang test.

LGTM

Another attempt to grab your attention :)

Hm, I would really say that __isnan and the other __ prefixed functions are Linuxisms, or more accurately, glibc-isms. They also don't exist on e.g. macOS:

Ping :)

N.B., EOWNERDEAD and ENOTRECOVERABLE are already defined on lines 158 and line 170, respectively.

Get rid of the ELAST trickery, which is hard to maintain, and does not
appear to serve any purpose. There is no mention of ELAST in the C or
C++ standards, as far as I know.

Now that I'm reading this header again, why do we even bother to define ELAST at all? On Linux, there is no such thing, while on BSDs and macOS, it is already provided by the regular errno.h.

Rewrite the __FreeBSD_version condition to be more straightforward.

Obsoleted by D58425 (and rCXX354515).

Convert m_(monitor|operation)_thread to llvm::Optional<>.

In rL365761#695543, @dim wrote:

@devnexen this appears to cause https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241137, where simply launching a process in lldb triggers a failure:

Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).

@devnexen this appears to cause https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241137, where simply launching a process in lldb triggers a failure:

For some reason this didn't get closed by Phabricator. Committed in rCRT374070.

@spatel, you mentioned this should be in 9.0.1, with "A noticeable perf regression for x86 vector code made it into the 9.0 release". Does it have a lot of influence on compile-time performance, or run-time performance? (I'd like to pull this one into FreeBSD's clang 9.0.)

In D68269#1691913, @ldionne wrote:

Pinging vendors for awareness of the change: @phosek @danalbert @dim

This is going in soon and will require a change in how you build libc++abi if you need new/delete definitions to be provided in libc++abi.so, namely to use -DLIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS=ON when you configure CMake for libc++abi.

The upshot of this change is that by default, we won't get both definitions in libc++ and libc++abi, thus unbreaking the default configuration.

LGTM from a FreeBSD point of view. :)

This works for me, and also for the original test case from https://bugs.freebsd.org/240764.

Third time's the charm.

Add _LIBCPP_C_HAS_NO_GETS macro to <__config>, and use that in <cstdio>.

Still OK with me :)

LGTM

LGTM. Maybe nice to merge it to 9.0 after a day or two.

In D65221#1600309, @vitalybuka wrote:

Is any test was broken?

In D55596#1599461, @arichardson wrote:

In D55596#1599456, @dim wrote:

In D55596#1599244, @arichardson wrote:

This broke ASAN on FreeBSD (same for the MSAN change). When loading static thread_local struct tsd_key key this is done using __tls_get_addr. The interceptor for __tls_get_addr then calls GetCurrentThread which calls AsanTSDGet which again calls __tls_get_addr.
If I remove the || SANITIZER_FREEBSD it works fine (at least on FreeBSD 11.2).

Yes indeed, this is https://bugs.llvm.org/show_bug.cgi?id=40761. I'm unsure what a good solution direction is.

I just spent some time debugging and it seems like https://reviews.llvm.org/D55596 works.

In D55596#1599244, @arichardson wrote:

This broke ASAN on FreeBSD (same for the MSAN change). When loading static thread_local struct tsd_key key this is done using __tls_get_addr. The interceptor for __tls_get_addr then calls GetCurrentThread which calls AsanTSDGet which again calls __tls_get_addr.
If I remove the || SANITIZER_FREEBSD it works fine (at least on FreeBSD 11.2).

FWIW, the original test case with pre-increment is fixed by this, e.g.:

No longer needed after rC362328 and follow-ups.

In fact, it is probably better to turn the OS check around, e.g. *only* increase the alignment for Linux, and nowhere else.

Please also exclude FreeBSD from these changes, since we care a lot about backwards compatibility, and specifically about alignment requirements. (We have run into many issues in our ports collection where upstream assumes everything is 16-byte aligned on i386, which is *NOT* ABI compliant.)

@efriedma any more work to be done on this? :)

So does this look better now?

Address review comments:

• Assign zero to pointed-to value in __kmp_store_mxcsr()
• Use SSE specific stuff in KMP_OS_UNIX part only