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

rustc: Update wasm32 support for LLVM 9 #62809

Merged
merged 2 commits into from
Jul 29, 2019
Merged

Conversation

alexcrichton
Copy link
Member

This commit brings in a number of minor updates for rustc's support for
the wasm target which has changed in the LLVM 9 update. Notable updates
include:

  • The compiler now no longer manually inserts the producers section,
    instead relying on LLVM to do so. LLVM uses the llvm.ident metadata
    for the processed-by directive (which is now emitted on the wasm
    target in this PR) and it uses debuginfo to figure out what language
    to put in the producers section.

  • Threaded WebAssembly code now requires different flags to be passed
    with LLD. In LLD we now pass:

    • --shared-memory - required since objects are compiled with
      atomics. This also means that the generated memory will be marked as
      shared.
    • --max-memory=1GB - required with the --shared-memory argument
      since shared memories in WebAssembly must have a maximum size. The
      1GB number is intended to be a conservative estimate for rustc, but
      it should be overridable with -C link-arg if necessary.
    • --passive-segments - this has become the default for multithreaded
      memory, but when compiling a threaded module all data segments need
      to be marked as passive to ensure they don't re-initialize memory
      for each thread. This will also cause LLD to emit a synthetic
      function to initialize memory which users will have to arrange to
      call.
    • The __heap_base and __data_end globals are explicitly exported
      since they're now hidden by default due to the --export flags we
      pass to LLD.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2019
@alexcrichton
Copy link
Member Author

cc @nikic

@alexcrichton
Copy link
Member Author

I've updated this as well with the necessary support to configure #[thread_local] on WebAssembly, meaning that the TLS implementation for wasm is much simplified and no longer needs custom external support to get something workable in libstd.

@rust-highfive

This comment has been minimized.

@alexcrichton alexcrichton force-pushed the wasm-llvm-9 branch 2 times, most recently from b307ccc to c696d57 Compare July 23, 2019 14:55
@rust-highfive

This comment has been minimized.

} else {
// stubbed out because no functions actually access these intrinsics
// unless atomics are enabled
MY_ID
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for using a compare_exchange loop here, rather than an atomic increment (something like MY_ID = NEXT_ID.fetch_add(1, SeqCst))? Is this not supported by wasm?

Copy link
Member Author

Choose a reason for hiding this comment

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

While perhaps slightly overzealous, the thinking here is that if the counter everything should deterministically panic, as opposed to only one thread deterministically panicking but others continuing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Maybe this is a nice opportunity to use the unstable fetch_update API, something like:

MY_ID = match NEXT_ID.fetch_update(|cur| cur.overflowing_add(1), SeqCst, SeqCst) {
    Some(id) => id + 1,
    Err(_) => crate::arch::wasm32::unreachable(),
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh good point! I think I'd prefer to avoid picking up an unstable feature for this since it's relatively minor, but I refactored a bit with your suggestion to use checked arithmetic which I think is better than what was there already!

@bors

This comment has been minimized.

This commit brings in a number of minor updates for rustc's support for
the wasm target which has changed in the LLVM 9 update. Notable updates
include:

* The compiler now no longer manually inserts the `producers` section,
  instead relying on LLVM to do so. LLVM uses the `llvm.ident` metadata
  for the `processed-by` directive (which is now emitted on the wasm
  target in this PR) and it uses debuginfo to figure out what `language`
  to put in the `producers` section.

* Threaded WebAssembly code now requires different flags to be passed
  with LLD. In LLD we now pass:

  * `--shared-memory` - required since objects are compiled with
    atomics. This also means that the generated memory will be marked as
    `shared`.
  * `--max-memory=1GB` - required with the `--shared-memory` argument
    since shared memories in WebAssembly must have a maximum size. The
    1GB number is intended to be a conservative estimate for rustc, but
    it should be overridable with `-C link-arg` if necessary.
  * `--passive-segments` - this has become the default for multithreaded
    memory, but when compiling a threaded module all data segments need
    to be marked as passive to ensure they don't re-initialize memory
    for each thread. This will also cause LLD to emit a synthetic
    function to initialize memory which users will have to arrange to
    call.
  * The `__heap_base` and `__data_end` globals are explicitly exported
    since they're now hidden by default due to the `--export` flags we
    pass to LLD.
@alexcrichton
Copy link
Member Author

Sorry to bother y'all again, but I'm not sure who's best to review this? @rust-lang/compiler would y'all be ok helping me find a reviewer for this?

@nikic
Copy link
Contributor

nikic commented Jul 25, 2019

r? @nikic I'll review a bit later today...

@rust-highfive rust-highfive assigned nikic and unassigned matthewjasper Jul 25, 2019
@rust-highfive

This comment has been minimized.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

r=me with the wrapping issue fixed.

I'm not familiar with wasm linking details, but it looks reasonable :)

src/libstd/sys/wasm/thread.rs Outdated Show resolved Hide resolved
} else {
// stubbed out because no functions actually access these intrinsics
// unless atomics are enabled
MY_ID
Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Maybe this is a nice opportunity to use the unstable fetch_update API, something like:

MY_ID = match NEXT_ID.fetch_update(|cur| cur.overflowing_add(1), SeqCst, SeqCst) {
    Some(id) => id + 1,
    Err(_) => crate::arch::wasm32::unreachable(),
}

src/librustc_target/spec/wasm32_base.rs Outdated Show resolved Hide resolved
This commit moves `thread_local!` on WebAssembly targets to using the
`#[thread_local]` attribute in LLVM. This was recently implemented
upstream and is [in the process of being documented][dox]. This change
only takes affect if modules are compiled with `+atomics` which is
currently unstable and a pretty esoteric method of compiling wasm
artifacts.

This "new power" of the wasm toolchain means that the old
`wasm-bindgen-threads` feature of the standard library can be removed
since it should now be possible to create a fully functioning threaded
wasm module without intrusively dealing with libstd symbols or
intrinsics. Yay!

[dox]: WebAssembly/tool-conventions#116
@alexcrichton
Copy link
Member Author

@bors: r=nikic

@bors
Copy link
Contributor

bors commented Jul 25, 2019

📌 Commit dc50a63 has been approved by nikic

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2019
@rust-highfive
Copy link
Collaborator

The job LinuxTools of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-07-25T18:26:24.2431296Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-07-25T18:26:24.2633298Z ##[command]git config gc.auto 0
2019-07-25T18:26:24.2709658Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-07-25T18:26:24.2756043Z ##[command]git config --get-all http.proxy
2019-07-25T18:26:24.2894515Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/62809/merge:refs/remotes/pull/62809/merge
---
2019-07-25T18:26:57.7362235Z do so (now or later) by using -b with the checkout command again. Example:
2019-07-25T18:26:57.7362262Z 
2019-07-25T18:26:57.7362646Z   git checkout -b <new-branch-name>
2019-07-25T18:26:57.7362673Z 
2019-07-25T18:26:57.7362715Z HEAD is now at 593ba213f Merge dc50a633f3260a3aeb79a4ca9800587be7f732e7 into eedf6ce4ef54bb03818ab21d714f1b9f13a6b31c
2019-07-25T18:26:57.7510806Z ##[section]Finishing: Checkout
2019-07-25T18:26:57.7518402Z ##[section]Starting: Decide whether to run this job
2019-07-25T18:26:57.7520877Z Task         : Bash
2019-07-25T18:26:57.7520935Z Description  : Run a Bash script on macOS, Linux, or Windows
2019-07-25T18:26:57.7521102Z Version      : 3.151.3
2019-07-25T18:26:57.7521140Z Author       : Microsoft Corporation
2019-07-25T18:26:57.7521140Z Author       : Microsoft Corporation
2019-07-25T18:26:57.7521200Z Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/bash
2019-07-25T18:26:57.7521244Z ==============================================================================
2019-07-25T18:26:57.8849822Z Generating script.
2019-07-25T18:26:57.8881342Z ========================== Starting Command Output ===========================
2019-07-25T18:26:57.8903554Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/6dcdbbcd-ac3f-4bed-9fea-8c929086552c.sh
2019-07-25T18:26:58.2097137Z Executing the job since submodules are updated
2019-07-25T18:26:58.2197431Z ##[section]Finishing: Decide whether to run this job
2019-07-25T18:26:58.2208298Z ==============================================================================
2019-07-25T18:26:58.2208353Z Task         : Bash
2019-07-25T18:26:58.2208417Z Description  : Run a Bash script on macOS, Linux, or Windows
2019-07-25T18:26:58.2208592Z Version      : 3.151.3
---
2019-07-25T19:53:04.9763051Z [ 69%] Building CXX object lib/Target/Mips/CMakeFiles/LLVMMipsCodeGen.dir/MipsISelDAGToDAG.cpp.o
2019-07-25T19:53:10.2308378Z [ 69%] Building CXX object lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SelectionDAGInfo.cpp.o
2019-07-25T19:53:12.4585443Z [ 69%] Building CXX object lib/Target/Mips/CMakeFiles/LLVMMipsCodeGen.dir/MipsISelLowering.cpp.o
2019-07-25T19:53:16.5634735Z [ 69%] Building CXX object lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o
2019-07-25T19:53:23.5003120Z [ 69%] Building CXX object lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64StackTagging.cpp.o
2019-07-25T19:53:30.2891621Z [ 69%] Building CXX object lib/Target/Mips/CMakeFiles/LLVMMipsCodeGen.dir/MipsLegalizerInfo.cpp.o
2019-07-25T19:53:31.2317445Z [ 69%] Building CXX object lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64StorePairSuppress.cpp.o
2019-07-25T19:53:36.3869472Z [ 69%] Building CXX object lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64Subtarget.cpp.o
2019-07-25T19:53:36.6958225Z [ 69%] Building CXX object lib/Target/Mips/CMakeFiles/LLVMMipsCodeGen.dir/MipsBranchExpansion.cpp.o
---
2019-07-25T21:43:11.6581966Z Cloning into 'rust-toolstate'...
2019-07-25T21:43:12.3090009Z The state of "rustc-guide" has changed from "test-pass" to ""
2019-07-25T21:43:12.3307706Z [master 6ffac73] (linux CI update)
2019-07-25T21:43:12.3309326Z  1 file changed, 1 insertion(+)
2019-07-25T21:43:12.9438737Z remote: Invalid username or password.
2019-07-25T21:43:12.9439619Z fatal: Authentication failed for 'https://github.com/rust-lang-nursery/rust-toolstate.git/'
2019-07-25T21:43:16.2460484Z  * branch            master     -> FETCH_HEAD
2019-07-25T21:43:16.2630045Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:16.2749892Z The state of "rustc-guide" has changed from "test-pass" to ""
2019-07-25T21:43:16.2906775Z [master 4e0c1f5] (linux CI update)
2019-07-25T21:43:16.2906775Z [master 4e0c1f5] (linux CI update)
2019-07-25T21:43:16.2907769Z  1 file changed, 1 insertion(+)
2019-07-25T21:43:16.5796432Z fatal: could not read Username for 'https://github.com': No such device or address
2019-07-25T21:43:18.9191678Z  * branch            master     -> FETCH_HEAD
2019-07-25T21:43:18.9332104Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:18.9435096Z The state of "rustc-guide" has changed from "test-pass" to ""
2019-07-25T21:43:18.9591663Z [master 666c8ef] (linux CI update)
2019-07-25T21:43:18.9591663Z [master 666c8ef] (linux CI update)
2019-07-25T21:43:18.9591741Z  1 file changed, 1 insertion(+)
2019-07-25T21:43:19.2355184Z fatal: could not read Username for 'https://github.com': No such device or address
2019-07-25T21:43:19.5427166Z  * branch            master     -> FETCH_HEAD
2019-07-25T21:43:19.5565835Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:19.5667530Z The state of "rustc-guide" has changed from "test-pass" to ""
2019-07-25T21:43:19.5828562Z [master 12cfd22] (linux CI update)
2019-07-25T21:43:19.5828562Z [master 12cfd22] (linux CI update)
2019-07-25T21:43:19.5829090Z  1 file changed, 1 insertion(+)
2019-07-25T21:43:19.8607832Z fatal: could not read Username for 'https://github.com': No such device or address
2019-07-25T21:43:22.1643726Z  * branch            master     -> FETCH_HEAD
2019-07-25T21:43:22.1776511Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:22.1876844Z The state of "rustc-guide" has changed from "test-pass" to ""
2019-07-25T21:43:22.2040255Z [master e40a318] (linux CI update)
2019-07-25T21:43:22.2040255Z [master e40a318] (linux CI update)
2019-07-25T21:43:22.2040679Z  1 file changed, 1 insertion(+)
2019-07-25T21:43:22.4798321Z fatal: could not read Username for 'https://github.com': No such device or address
2019-07-25T21:43:24.7865019Z  * branch            master     -> FETCH_HEAD
2019-07-25T21:43:24.8041671Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:24.8041671Z HEAD is now at 994f84a (windows CI update)
2019-07-25T21:43:25.4853774Z ##[error]Bash exited with code '1'.
2019-07-25T21:43:25.4892815Z ##[section]Starting: Checkout
2019-07-25T21:43:25.4894881Z ==============================================================================
2019-07-25T21:43:25.4894943Z Task         : Get sources
2019-07-25T21:43:25.4894995Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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 @TimNN. (Feature Requests)

Centril added a commit to Centril/rust that referenced this pull request Jul 28, 2019
rustc: Update wasm32 support for LLVM 9

This commit brings in a number of minor updates for rustc's support for
the wasm target which has changed in the LLVM 9 update. Notable updates
include:

* The compiler now no longer manually inserts the `producers` section,
  instead relying on LLVM to do so. LLVM uses the `llvm.ident` metadata
  for the `processed-by` directive (which is now emitted on the wasm
  target in this PR) and it uses debuginfo to figure out what `language`
  to put in the `producers` section.

* Threaded WebAssembly code now requires different flags to be passed
  with LLD. In LLD we now pass:

  * `--shared-memory` - required since objects are compiled with
    atomics. This also means that the generated memory will be marked as
    `shared`.
  * `--max-memory=1GB` - required with the `--shared-memory` argument
    since shared memories in WebAssembly must have a maximum size. The
    1GB number is intended to be a conservative estimate for rustc, but
    it should be overridable with `-C link-arg` if necessary.
  * `--passive-segments` - this has become the default for multithreaded
    memory, but when compiling a threaded module all data segments need
    to be marked as passive to ensure they don't re-initialize memory
    for each thread. This will also cause LLD to emit a synthetic
    function to initialize memory which users will have to arrange to
    call.
  * The `__heap_base` and `__data_end` globals are explicitly exported
    since they're now hidden by default due to the `--export` flags we
    pass to LLD.
bors added a commit that referenced this pull request Jul 28, 2019
Rollup of 4 pull requests

Successful merges:

 - #62550 (Implement RFC 2707 + Parser recovery for range patterns)
 - #62759 (Actually add rustc-guide to toolstate, don't fail builds for the guide)
 - #62809 (rustc: Update wasm32 support for LLVM 9)
 - #62974 (bump crossbeam-epoch dependency)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jul 29, 2019

⌛ Testing commit dc50a63 with merge e14bb100ce50095b35791513814b3cbdd10ea862...

Centril added a commit to Centril/rust that referenced this pull request Jul 29, 2019
rustc: Update wasm32 support for LLVM 9

This commit brings in a number of minor updates for rustc's support for
the wasm target which has changed in the LLVM 9 update. Notable updates
include:

* The compiler now no longer manually inserts the `producers` section,
  instead relying on LLVM to do so. LLVM uses the `llvm.ident` metadata
  for the `processed-by` directive (which is now emitted on the wasm
  target in this PR) and it uses debuginfo to figure out what `language`
  to put in the `producers` section.

* Threaded WebAssembly code now requires different flags to be passed
  with LLD. In LLD we now pass:

  * `--shared-memory` - required since objects are compiled with
    atomics. This also means that the generated memory will be marked as
    `shared`.
  * `--max-memory=1GB` - required with the `--shared-memory` argument
    since shared memories in WebAssembly must have a maximum size. The
    1GB number is intended to be a conservative estimate for rustc, but
    it should be overridable with `-C link-arg` if necessary.
  * `--passive-segments` - this has become the default for multithreaded
    memory, but when compiling a threaded module all data segments need
    to be marked as passive to ensure they don't re-initialize memory
    for each thread. This will also cause LLD to emit a synthetic
    function to initialize memory which users will have to arrange to
    call.
  * The `__heap_base` and `__data_end` globals are explicitly exported
    since they're now hidden by default due to the `--export` flags we
    pass to LLD.
@Centril
Copy link
Contributor

Centril commented Jul 29, 2019

@bors retry rolled up.

bors added a commit that referenced this pull request Jul 29, 2019
Rollup of 6 pull requests

Successful merges:

 - #62809 (rustc: Update wasm32 support for LLVM 9)
 - #63055 (Various cleanups to save analysis)
 - #63076 (Miri: fix determining size of an "extra function" allocation)
 - #63077 (cleanup: Remove some language features related to built-in macros)
 - #63086 (Ignore test cases that are not supported by vxWorks)
 - #63092 (Update `impl Trait` gate issues)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jul 29, 2019

⌛ Testing commit dc50a63 with merge 8b94e9e...

@bors bors merged commit dc50a63 into rust-lang:master Jul 29, 2019
@alexcrichton alexcrichton deleted the wasm-llvm-9 branch July 31, 2019 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants