-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Update emscripten #55626
Update emscripten #55626
Conversation
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
|
Per the discussion on internals, I think we could also drop LLVM 5 now. I do still need support for LLVM 6 for external LLVM though, and of course that's what you have here for emscripten too. |
1343839
to
520a4f0
Compare
asmjs builder succeeded: https://travis-ci.org/rust-lang/rust/builds/449986971 I've dropped the travis.yml changes now, so this should be good to go. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
124e8d4
to
b65457e
Compare
📌 Commit b65457e23d1d2ff82bee0675e64fc611d4e27fb7 has been approved by |
@cuviper I'll send a PR to update the minimum LLVM version 6 once this is merged. |
☔ The latest upstream changes (presumably #55238) made this pull request unmergeable. Please resolve the merge conflicts. |
b65457e
to
8b19578
Compare
Rebased :) |
@bors: r+ |
📌 Commit 8b195780729c5f3388b6d6fe51feb8f9d0c6afc7 has been approved by |
☔ The latest upstream changes (presumably #55349) made this pull request unmergeable. Please resolve the merge conflicts. |
This updates emscripten to version 1.38.15, which is based on LLVM 6.0.1.
Use eprintln!() to make sure stdio is flushed.
Either missing i128 or asm support
Oh dear that's unfortunate :( So that comes up because that builder (and Due to how ancient these containers are, though, no one really make sure code compiles in them any more. This means that C/C++ projects can often run afoul of very obscure errors (like the above) which happen in no modern environments. This is why we carry a few build-related patches on our LLVM fork as they're only applicable in these ancient environments. In theory this is ideally fixed by sending a patch to upstream LLVM but I'm not sure that makes sense because not many would really benefit from this other than us (and it may be too onerous a patch to send upstream). If we can figure out workarounds to codify in the containers that'd be great, but otherwise our last resort is just another branch on our LLVM repo |
Could you just add `CPPFLAGS=-DPATH_MAX=4096`? Or might need `CFLAGS` or
`CXXFLAGS` instead, depending on how the LLVM build applies flags.
|
📌 Commit 82574e9 has been approved by |
Updated submodule to https://github.com/rust-lang/llvm/commits/rust-emscripten-2018-11-09 with a patch for missing PATH_MAX.
This issue has already been fixed upstream by llvm-mirror/llvm@612ed3c, but this was after some significant refactorings to the code, so this fix is not directly applicable to LLVM 6.0.1. |
For anyone else who is wondering about this, found an old internals thread discussing the glibc requirement: https://internals.rust-lang.org/t/bumping-glibc-requirements-for-the-rust-toolchain/5111 |
Update emscripten This updates emscripten to 1.38.15, which is based on LLVM 6.0.1 and would allow us to drop code for handling LLVM 4. The main issue I ran into is that exporting statics through `EXPORTED_FUNCTIONS` no longer works. As far as I understand exporting non-functions doesn't really make sense under emscripten anyway, so I've modified the symbol export code to not even try. Closes #52323.
💔 Test failed - status-appveyor |
Looking at a difference in step timings between that build and the last successful one:
it looks like... everything was just slower AFAIK nothing here would slow down everything that much, so gonna try chalking this up to CI ... @bors: retry |
Update emscripten This updates emscripten to 1.38.15, which is based on LLVM 6.0.1 and would allow us to drop code for handling LLVM 4. The main issue I ran into is that exporting statics through `EXPORTED_FUNCTIONS` no longer works. As far as I understand exporting non-functions doesn't really make sense under emscripten anyway, so I've modified the symbol export code to not even try. Closes #52323.
☀️ Test successful - status-appveyor, status-travis |
Thank you ! So for clarification, the LLVM version uses by |
@gnzlbg Yep, that's right. We support three emscripten based targets, namely asmjs-unknown-emscripten, wasm32-unknown-emscripten and wasm32-experimental-emscripten. For the first two emscripten currently requires their LLVM 6.0.1 fork (which is what is updated here), while the latter uses the native LLVM wasm backend. As we still support the first two targets, we can't upgrade past LLVM 6.0.1 yet. |
…crichton Set BINARYEN_TRAP_MODE=clamp This fixes the wasm32-unknown-emscripten test failure mentioned in rust-lang#55626 (comment), by making binaryen operate in clamp rather than trap mode. The issue is that the current `-Zsaturating-float-casts` implementation uses `fpto[us]i` unconditionally (and selects afterwards), which does not work with trapping implementations of fpto[su]i, which emscripten uses by default. I've left a FIXME to drop this flag once we have a better solution for saturating casts on the LLVM side. ;
Remove support for building against LLVM 4 With emscripten removed in #55626, we no longer need to support building against LLVM 4.
This updates emscripten to 1.38.15, which is based on LLVM 6.0.1 and would allow us to drop code for handling LLVM 4.
The main issue I ran into is that exporting statics through
EXPORTED_FUNCTIONS
no longer works. As far as I understand exporting non-functions doesn't really make sense under emscripten anyway, so I've modified the symbol export code to not even try.Closes #52323.