WASM backend cannot build libcxx - fails on AtomicLoad

[Quuxplusone]

Bugzilla Link	PR35411
Status	RESOLVED FIXED
Importance	P enhancement
Reported by	Nicholas Wilson (ncw@realvnc.com)
Reported on	2017-11-24 06:02:01 -0800
Last modified on	2017-11-28 03:41:13 -0800
Version	trunk
Hardware	PC Linux
CC	llvm-bugs@lists.llvm.org, llvm@sunfishcode.online, ncw@realvnc.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

I have to say, it's taken quite a while to come up with the right toolchain, so
that CMake is able to work with the libcxx build system!

However, the error I'm getting below seems to be a problem with the WASM
backend, and isn't related to libcxx itself (I don't think).  It looks like the
__atomic_load_n builtin just doesn't work with the WASM backend.

### Build error ###

Building CXX object lib/CMakeFiles/cxx_objects.dir/__/src/algorithm.cpp.obj
In file included from /home/ncw/workspace/llvm/libcxx/src/algorithm.cpp:11:
In file included from /home/ncw/workspace/llvm/libcxx/include/random:1646:
/home/ncw/workspace/llvm/libcxx/include/istream:714:24: warning: comparison
'long' < -2147483648 is always false [-Wtautological-constant-compare]
            if (__temp < numeric_limits<int>::min())
                ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ncw/workspace/llvm/libcxx/include/istream:719:29: warning: comparison
'long' > 2147483647 is always false [-Wtautological-constant-compare]
            else if (__temp > numeric_limits<int>::max())
                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~
fatal error: error in backend: Cannot select: t19: i32,ch = AtomicLoad<Volatile
LD1[bitcast (i32* @_ZGVZNSt3__112__rs_defaultclEvE6__rs_g to i8*)](align=4)>
      t0, t25
  t25: i32 = WebAssemblyISD::Wrapper TargetGlobalAddress:i32<i32* @_ZGVZNSt3__112__rs_defaultclEvE6__rs_g> 0
    t24: i32 = TargetGlobalAddress<i32* @_ZGVZNSt3__112__rs_defaultclEvE6__rs_g> 0
In function: _ZNSt3__112__rs_defaultclEv
clang-6.0: error: clang frontend command failed with exit code 70 (use -v to
see invocation)
clang version 6.0.0 (trunk 318652)
Target: wasm32-unknown-unknown-wasm
Thread model: posix
InstalledDir: /home/ncw/workspace/llvm/compile-root/bin
clang-6.0: note: diagnostic msg: PLEASE submit a bug report to
http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and
associated run script.
clang-6.0: note: diagnostic msg:
********************

I could post the full source of the failing libcxx file, but I'll see first if
I can distill something more minimal!

### How I configured libcxx ###

I have written some additions to Musl which allow it to build for Wasm. The C
side of things is fine. Now I'm moving on to C++, building libcxx first.  I've
installed the Wasm version of Musl to a sysroot, and also installed there the
Clang configured to default to "--target=wasm32-unknown-unknown-wasm".

I configured libcxx as follows:

cmake -DCMAKE_TOOLCHAIN_FILE=~/workspace/llvm/toolchain-libcxx-bootstrap.cmake -
DLLVM_PATH=../../llvm -DLIBCXX_CXX_ABI=libcxxabi -
DLIBCXX_CXX_ABI_INCLUDE_PATHS=../../libcxxabi/include -
DCMAKE_INSTALL_PREFIX=/home/ncw/workspace/llvm/compile-root -
DLIBCXX_ENABLE_SHARED=OFF -DLIBCXX_ENABLE_THREADS=OFF -
DLIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE=OFF -DLIBCXX_HAS_MUSL_LIBC=ON -
DCMAKE_BUILD_TYPE=MinSizeRel

The CMake toolchain needs a number of hacks, because it runs clang++ to detect
many different things, but we don't as yet have a working C++ compiler (since
we're trying to build libcxx first). With some hacks, it does get itself off
the ground however.

[Quuxplusone]

### Minimal Example ###

== compile-error-atomic.cxx ==
extern void extFn();

namespace {
  struct DummyStruct {
    DummyStruct() { extFn(); }
  };
}

void expFn()
{
  static DummyStruct dummy;
}

== To build ==

> clang++ -o /dev/null compile-error-atomic.cxx

== Output ==

fatal error: error in backend: Cannot select: t10: i32,ch = AtomicLoad<Volatile
LD1[bitcast (i32* @_ZGVZ5expFnvE5dummy to i8*)](align=4)> t0, t13
  t13: i32 = WebAssemblyISD::Wrapper TargetGlobalAddress:i32<i32* @_ZGVZ5expFnvE5dummy> 0
    t12: i32 = TargetGlobalAddress<i32* @_ZGVZ5expFnvE5dummy> 0
In function: _Z5expFnv
clang-6.0: error: clang frontend command failed with exit code 70 (use -v to
see invocation)
clang version 6.0.0 (trunk 318652)
Target: wasm32-unknown-unknown-wasm
Thread model: posix
InstalledDir: /home/ncw/workspace/llvm/compile-root/bin
clang-6.0: note: diagnostic msg: PLEASE submit a bug report to
http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and
associated run script.

[Quuxplusone]

I've been having a go at fixing this.

Some observations:

1. The atomic_load comes from ItaniumCXXABI::EmitGuardedInit
2. However, if you emit an atomic load using the "__atomic_load" intrinsic
   directly in the source code, then everything is fine.
3. WebAssemblyPassConfig::addIRPasses has code for handling the situation, and
   can switch between POSIX/single-threaded mode using "-mthread-model single",
   and with that in place the static initialiser actually compiles.

I'm uncertain how to go ahead fixing it.

a) Should Webassembly default to using ThreadModel=Single? That would be
   consistent, and currently it's clear that Wasm can't really support anything
   else.
b) ItaniumCXXABI has an option "ThreadsafeStatics" ("-fno-threadsafe-statics")
   that controls whether the offending atomic operations are emitted in the
   first place.  Should that be set to false, when the target processor is
   using ThreadModel=Single, or should ItaniumCXXABI check the ThreadModel as
   well as the ThreadsafeStatics setting?
c) Finally, it could be fixed in the WebAssembly lowering code - clearly there's
   another pass of lowering going on, which is able to lower direct use of an
   "__atomic_load" intrinsic, but somehow it just isn't kicking in to lower the
   AtomicLoad emitted by the threadsafe-static-initialiser.

[Quuxplusone]

So far, having users that encounter this add "-mthread-model single" to their compile commands has been sufficient. When WebAssembly gets threads, we'll just implement all the atomics, and this issue will be fixed.

"-mthread-model single" isn't currently enabled by default because it hasn't come up very often, and when it has, it's been convenient to discover when code being compiled thinks it's being compiled multi-threaded. And it'll mean we won't silently break anything when we do implement atomics.

It does seem like -fno-threadsafe-statics could be enabled by "-mthread-model single", though that seems like an independent optimization rather than a real fix.

[Quuxplusone]

Thanks for considering it.  Since there's a workaround, it *could* be left
until Atomics.

However, this does render C++ pretty-much unusable, since a function-local
static is a construct that is widely-used even in single-threaded code.

I don't know when Atomics are arriving, but if it's more than a couple of
months you might want to make Clang work out of the box with the C++ in the
meantime.

As as aside, I'm currently hacking away at patches for some of the other C++
sticking points, like inlines (aka COMDAT support).  All of these things that
need to be implemented don't seem to be tracked here in bugzilla or in any of
the "official" Wasm github projects I can find.  Is there some place where this
work is actually being organised, so I can see what other people are working on
at the moment?

[Quuxplusone]

(In reply to Nicholas Wilson from comment #4)
> Thanks for considering it.  Since there's a workaround, it *could* be left
> until Atomics.
>
> However, this does render C++ pretty-much unusable, since a function-local
> static is a construct that is widely-used even in single-threaded code.
>
> I don't know when Atomics are arriving, but if it's more than a couple of
> months you might want to make Clang work out of the box with the C++ in the
> meantime.

That's a good point. It hasn't really been an issue until recently, but with
everything that's happening recently, I think it does make sense to do this by
default now.

> As as aside, I'm currently hacking away at patches for some of the other C++
> sticking points, like inlines (aka COMDAT support).  All of these things
> that need to be implemented don't seem to be tracked here in bugzilla or in
> any of the "official" Wasm github projects I can find.  Is there some place
> where this work is actually being organised, so I can see what other people
> are working on at the moment?

We discuss most of the big LLVM wasm backend patches on the review tracker
https://reviews.llvm.org/, such as https://reviews.llvm.org/D39873 which
recently disabled COMDAT support (which you may want to re-enable if you're
working on that :-)). And now that wasm lld support has moved upstream, that
should include major lld patches as well.

https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md is the
work-in-progress document that describes the extra metadata to describe
relocations and additional linking features. So that's a place where we can
file issues and PRs and discuss them.

[Quuxplusone]

r319101 enables -mthread-model single by default.

[Quuxplusone]

Great, thanks for that change, and for the pointers. Things are moving fast on this project!

Resolving as FIXED since my issue is sorted for now.