Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Emscripten targets to use upstream LLVM backend #63649

Merged
merged 7 commits into from
Oct 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@
# but you can also optionally enable the "emscripten" backend for asm.js or
# make this an empty array (but that probably won't get too far in the
# bootstrap)
# FIXME: remove the obsolete emscripten backend option.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the codegen-backends option when you remove that.

#codegen-backends = ["llvm"]

# This is the name of the directory in which codegen backends will get installed
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,7 @@ impl<'a> Builder<'a> {
Some("-Wl,-rpath,@loader_path/../lib")
} else if !target.contains("windows") &&
!target.contains("wasm32") &&
!target.contains("emscripten") &&
!target.contains("fuchsia") {
Some("-Wl,-rpath,$ORIGIN/../lib")
} else {
Expand Down
5 changes: 3 additions & 2 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,10 +1047,11 @@ impl Step for Compiletest {
// Also provide `rust_test_helpers` for the host.
builder.ensure(native::TestHelpers { target: compiler.host });

// wasm32 can't build the test helpers
if !target.contains("wasm32") {
// As well as the target, except for plain wasm32, which can't build it
if !target.contains("wasm32") || target.contains("emscripten") {
builder.ensure(native::TestHelpers { target });
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure which LLVM versions rust supports, so this may not be correct. This change was necessary to uncover all the ABI problems, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this may fail on CI for the wasm32-unknown-unknown target, but I think that it'll probably work in that it'll generate a native library with ELF code (using clang) and then we'd just ignore it during the test suite. In that sense let's just ignore this for now and if it causes problems on CI we can add it back, but only for the non-emscripten target

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is the line that's failing on CI, this'll need to be disabled still for wasm32-unknown-unknown I believe


builder.ensure(RemoteCopyLibs { compiler, target });

let mut cmd = builder.tool_cmd(Tool::Compiletest);
Expand Down
29 changes: 9 additions & 20 deletions src/ci/docker/asmjs/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
cmake \
sudo \
gdb \
xz-utils
xz-utils \
bzip2

COPY scripts/emscripten.sh /scripts/
RUN bash /scripts/emscripten.sh
Expand All @@ -20,28 +21,16 @@ COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

ENV PATH=$PATH:/emsdk-portable
ENV PATH=$PATH:/emsdk-portable/clang/e1.38.15_64bit/
ENV PATH=$PATH:/emsdk-portable/emscripten/1.38.15/
ENV PATH=$PATH:/emsdk-portable/node/8.9.1_64bit/bin/
ENV EMSCRIPTEN=/emsdk-portable/emscripten/1.38.15/
ENV BINARYEN_ROOT=/emsdk-portable/clang/e1.38.15_64bit/binaryen/
ENV PATH=$PATH:/emsdk-portable/upstream/emscripten/
ENV PATH=$PATH:/emsdk-portable/node/12.9.1_64bit/bin/
ENV BINARYEN_ROOT=/emsdk-portable/upstream/
ENV EM_CONFIG=/emsdk-portable/.emscripten

ENV TARGETS=asmjs-unknown-emscripten

ENV RUST_CONFIGURE_ARGS --enable-emscripten --disable-optimize-tests
ENV SCRIPT python2.7 ../x.py test --target $TARGETS

ENV SCRIPT python2.7 ../x.py test --target $TARGETS \
src/test/ui \
src/test/run-fail \
src/libstd \
src/liballoc \
src/libcore

# Debug assertions in rustc are largely covered by other builders, and LLVM
# assertions cause this builder to slow down by quite a large amount and don't
# buy us a huge amount over other builders (not sure if we've ever seen an
# asmjs-specific backend assertion trip), so disable assertions for these
# tests.
ENV NO_LLVM_ASSERTIONS=1
# This is almost identical to the wasm32-unknown-emscripten target, so
# running with assertions again is not useful
ENV NO_DEBUG_ASSERTIONS=1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think we trimmed down the tests run on the emscripten builder because of how long it took, so this may want to preserve the lack of assertions and running just a few test suites

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hope is that with the upstream LLVM backend instead of Fastcomp the performance should be similar to that of other targets so we can give it the full testing treatment, but I'd be happy to put these limits back if that does not pan out.

ENV NO_LLVM_ASSERTIONS=1
35 changes: 0 additions & 35 deletions src/ci/docker/disabled/wasm32-exp/Dockerfile

This file was deleted.

9 changes: 0 additions & 9 deletions src/ci/docker/disabled/wasm32-exp/node.sh

This file was deleted.

22 changes: 14 additions & 8 deletions src/ci/docker/disabled/wasm32/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,28 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
cmake \
sudo \
gdb \
xz-utils
xz-utils \
bzip2

# emscripten
COPY scripts/emscripten.sh /scripts/
RUN bash /scripts/emscripten.sh

COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

ENV PATH=$PATH:/emsdk-portable
ENV PATH=$PATH:/emsdk-portable/clang/e1.38.15_64bit/
ENV PATH=$PATH:/emsdk-portable/emscripten/1.38.15/
ENV PATH=$PATH:/emsdk-portable/node/8.9.1_64bit/bin/
ENV EMSCRIPTEN=/emsdk-portable/emscripten/1.38.15/
ENV BINARYEN_ROOT=/emsdk-portable/clang/e1.38.15_64bit/binaryen/
ENV PATH=$PATH:/emsdk-portable/upstream/emscripten/
ENV PATH=$PATH:/emsdk-portable/node/12.9.1_64bit/bin/
ENV BINARYEN_ROOT=/emsdk-portable/upstream/
ENV EM_CONFIG=/emsdk-portable/.emscripten

ENV TARGETS=wasm32-unknown-emscripten
ENV SCRIPT python2.7 ../x.py test --target $TARGETS

# FIXME: Re-enable these tests once Cargo stops trying to execute wasms
ENV SCRIPT python2.7 ../x.py test --target $TARGETS \
--exclude src/libcore \
--exclude src/liballoc \
--exclude src/libproc_macro \
--exclude src/libstd \
--exclude src/libterm \
--exclude src/libtest
37 changes: 0 additions & 37 deletions src/ci/docker/scripts/emscripten-wasm.sh

This file was deleted.

11 changes: 3 additions & 8 deletions src/ci/docker/scripts/emscripten.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,15 @@ exit 1
set -x
}

cd /
curl -fL https://mozilla-games.s3.amazonaws.com/emscripten/releases/emsdk-portable.tar.gz | \
tar -xz

git clone https://github.com/emscripten-core/emsdk.git /emsdk-portable
cd /emsdk-portable
./emsdk update
hide_output ./emsdk install sdk-1.38.15-64bit
./emsdk activate sdk-1.38.15-64bit
hide_output ./emsdk install 1.38.46-upstream
./emsdk activate 1.38.46-upstream

# Compile and cache libc
source ./emsdk_env.sh
echo "main(){}" > a.c
HOME=/emsdk-portable/ emcc a.c
HOME=/emsdk-portable/ emcc -s BINARYEN=1 a.c
rm -f a.*

# Make emsdk usable by any user
Expand Down
13 changes: 7 additions & 6 deletions src/liballoc/tests/binary_heap.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
use std::cmp;
use std::collections::BinaryHeap;
use std::collections::binary_heap::{Drain, PeekMut};
use std::panic::{self, AssertUnwindSafe};
use std::sync::atomic::{AtomicUsize, Ordering};

use rand::{thread_rng, seq::SliceRandom};

#[test]
fn test_iterator() {
Expand Down Expand Up @@ -281,9 +276,15 @@ fn assert_covariance() {
// even if the order may not be correct.
//
// Destructors must be called exactly once per element.
// FIXME: re-enable emscripten once it can unwind again
#[test]
#[cfg(not(miri))] // Miri does not support catching panics
#[cfg(not(any(miri, target_os = "emscripten")))] // Miri does not support catching panics
fn panic_safe() {
use std::cmp;
use std::panic::{self, AssertUnwindSafe};
use std::sync::atomic::{AtomicUsize, Ordering};
use rand::{thread_rng, seq::SliceRandom};

static DROP_COUNTER: AtomicUsize = AtomicUsize::new(0);

#[derive(Eq, PartialEq, Ord, Clone, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion src/liballoc/tests/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ mod slice_index {
}

#[test]
#[cfg(not(target_arch = "asmjs"))] // hits an OOM
#[cfg(not(target_os = "emscripten"))] // hits an OOM
#[cfg(not(miri))] // Miri is too slow
fn simple_big() {
fn a_million_letter_x() -> String {
Expand Down
10 changes: 10 additions & 0 deletions src/liballoc/tests/str.rs.rej
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
diff a/src/liballoc/tests/str.rs b/src/liballoc/tests/str.rs (rejected hunks)
@@ -483,7 +483,7 @@ mod slice_index {
}

#[test]
- #[cfg(not(target_arch = "asmjs"))] // hits an OOM
+ #[cfg(not(target_arch = "js"))] // hits an OOM
#[cfg(not(miri))] // Miri is too slow
fn simple_big() {
fn a_million_letter_x() -> String {
7 changes: 5 additions & 2 deletions src/liballoc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,10 @@ fn drain_filter_complex() {
}
}

// Miri does not support catching panics
// FIXME: re-enable emscripten once it can unwind again
#[test]
#[cfg(not(miri))] // Miri does not support catching panics
#[cfg(not(any(miri, target_os = "emscripten")))]
fn drain_filter_consumed_panic() {
use std::rc::Rc;
use std::sync::Mutex;
Expand Down Expand Up @@ -995,8 +997,9 @@ fn drain_filter_consumed_panic() {
}
}

// FIXME: Re-enable emscripten once it can catch panics
#[test]
#[cfg(not(miri))] // Miri does not support catching panics
#[cfg(not(any(miri, target_os = "emscripten")))] // Miri does not support catching panics
fn drain_filter_unconsumed_panic() {
use std::rc::Rc;
use std::sync::Mutex;
Expand Down
56 changes: 18 additions & 38 deletions src/libcore/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ impl fmt::Debug for c_void {
/// Basic implementation of a `va_list`.
// The name is WIP, using `VaListImpl` for now.
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
target_arch = "wasm32",
target_arch = "asmjs",
windows))]
#[repr(transparent)]
#[unstable(feature = "c_variadic",
Expand All @@ -67,8 +69,10 @@ pub struct VaListImpl<'f> {
}

#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
target_arch = "wasm32",
target_arch = "asmjs",
windows))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
Expand Down Expand Up @@ -137,38 +141,6 @@ pub struct VaListImpl<'f> {
_marker: PhantomData<&'f mut &'f c_void>,
}

/// asm.js ABI implementation of a `va_list`.
// asm.js uses the PNaCl ABI, which specifies that a `va_list` is
// an array of 4 32-bit integers, according to the old PNaCl docs at
// https://web.archive.org/web/20130518054430/https://www.chromium.org/nativeclient/pnacl/bitcode-abi#TOC-Derived-Types
// and clang does the same in `CreatePNaClABIBuiltinVaListDecl` from `lib/AST/ASTContext.cpp`
#[cfg(all(target_arch = "asmjs", not(windows)))]
#[repr(C)]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
inner: [crate::mem::MaybeUninit<i32>; 4],
_marker: PhantomData<&'f mut &'f c_void>,
}

#[cfg(all(target_arch = "asmjs", not(windows)))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'f> fmt::Debug for VaListImpl<'f> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
unsafe {
write!(f, "va_list* [{:#x}, {:#x}, {:#x}, {:#x}]",
self.inner[0].read(), self.inner[1].read(),
self.inner[2].read(), self.inner[3].read())
}
}
}

/// A wrapper for a `va_list`
#[repr(transparent)]
#[derive(Debug)]
Expand All @@ -178,23 +150,29 @@ impl<'f> fmt::Debug for VaListImpl<'f> {
issue = "44930")]
pub struct VaList<'a, 'f: 'a> {
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
target_arch = "wasm32",
target_arch = "asmjs",
windows))]
inner: VaListImpl<'f>,

#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
target_arch = "x86_64", target_arch = "asmjs"),
target_arch = "x86_64"),
any(not(target_arch = "aarch64"), not(target_os = "ios")),
not(target_arch = "wasm32"),
not(target_arch = "asmjs"),
not(windows)))]
inner: &'a mut VaListImpl<'f>,

_marker: PhantomData<&'a mut VaListImpl<'f>>,
}

#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
target_arch = "wasm32",
target_arch = "asmjs",
windows))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
Expand All @@ -212,8 +190,10 @@ impl<'f> VaListImpl<'f> {
}

#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
target_arch = "x86_64", target_arch = "asmjs"),
target_arch = "x86_64"),
any(not(target_arch = "aarch64"), not(target_os = "ios")),
not(target_arch = "wasm32"),
not(target_arch = "asmjs"),
not(windows)))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
Expand Down
Loading