[PATCH] D59676: Make Parallel.h build with libc++ on Windows.

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

In D59676#1438927 <https://reviews.llvm.org/D59676#1438927>, @BillyONeal wrote:

>


Thanks for chiming in!

> Attempting to use ConcRT powered machinery (like the Concurrency:: primitives in this file) with a different STL like libc++ is likely to be non-functional, since that calls into concrt140.dll which assumes MSVC++'s STL.

'assumes' in what way? If concrt140.dll uses msvcp140.dll internally but no STL objects are passed from user code (built with libc++) to ppl functions to concrt140.dll, things should be fine for example.

(libc++ has all its symbols in an inline namespace, so you don't get symbol collisions and can have e.g. libc++ and libstdc++ in the same process. From what I understand, COFF is less symbol-name-based than ELF and there are fewer issues around the symbol name problem there anyways.)

> There's also an issue here that this code is using std::mutex with ConcRT powered machinery like Concurrency::parallel_for_each, which is not supported and is likely to result in deadlocks. ConcRT assumes that everything you give to it is non-blocking, so you can't use Concurrency::parallel_for_each with platform synchronization primitives. (This is the biggest reason we, MSVC++, are moving away from ConcRT as a technoligy and did not use it to power our parallel algorithms implementation)

I filed https://bugs.llvm.org/show_bug.cgi?id=41198 for this part. Since that's a problem with the normal MSVC standard library, this seems like the biggest issue here. (But in practice, things seem to work fine so far.) But it's also independent of the libc++ part.

> There's an additional concern here that ppl.h (and yvals.h) will force linking with MSVC++'s STL DLL, msvcp140.dll, because support machinery for ppl.h live in there. yvals.h enforces this with pragma comment(lib, "msvcp140.dll") et al. yvals.h also emits corresponding pragma detect_mismatch directives which libc++ probably doesn't want to expose.

IIRC we need bits of the lower-level part of MSVC++'s standard library for abi things like stdexcept (which in llvm land is in libc++abi and not libc++, but it's in the same library in Windows). I don't remember the details, but I can look them up if necessary.

> I'm not really sure what the path forward here is.

Two things:

1. For Chromium, we don't need to build clang and lld etc (for building chrome) with libc++. We do have a fork of LLVM 7.0 that's used by some GPU shader software emulation layer that we ship, and we need to build that with libc++ – but it's probably fine to build with LLVM_ENABLE_THREADS to sidestep this issue.

2. For LLVM built with libc++ (which would just abstractly nice if it worked), I think it depends a bit on what exactly 'likely non-functional' above means :) Maybe it happens to work, and then we can land this patch for now. The concrt + mutex incompat issue might mean we have to move off concrt eventually (independent of libc++), which would then also solve this issue here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59676/new/

https://reviews.llvm.org/D59676