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

Rollup of 7 pull requests #83864

Merged
merged 20 commits into from
Apr 5, 2021
Merged

Rollup of 7 pull requests #83864

merged 20 commits into from
Apr 5, 2021

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

notriddle and others added 20 commits March 31, 2021 12:44
This is step 2 towards fixing rust-lang#77548.

In the codegen and codegen-units test suites, the `//` comment markers
were kept in order not to affect any source locations. This is because
these tests cannot be automatically `--bless`ed.
Co-authored-by: Yuki Okushi <jtitor@2k36.org>
Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.
Co-authored-by: kennytm <kennytm@gmail.com>
wasm64 support

There is still some upstream llvm work needed before this can land.
core: disable `ptr::swap_nonoverlapping_one`'s block optimization on SPIR-V.

SPIR-V primarily supports what it calls the "Logical addressing model" (and AFAIK for graphical shaders it's the only option), and what that implies is that there is no "memory" to uniformly address at some byte/word level, and that you can't really talk about values having a "raw representation" in terms of sequences of bytes. Therefore, the "block"-wise swapping optimization employed by `ptr::swap_nonoverlapping_one` (where a "block" is 32 bytes, currently), is fundamentally incompatible with SPIR-V "memory".

As such, [Rust-GPU](https://github.com/EmbarkStudios/rust-gpu/)'s `rustc_codegen_spirv` backend cannot currently allow the use of `ptr::swap_nonoverlapping_one` - but that comes at a great price, since it's the building block of `mem::{swap,replace}`, and those in turn are used by e.g. `Option::take` and `Range`'s `Iterator` implementation (the latter blocking the use of `for i in 0..n` loops).

There's 4 options I can see in terms of supporting `ptr::swap_nonoverlapping_one` in `rustc_codegen_spirv`:
* legalize the block-wise swap loop back into swapping whole values, for SPIR-V
  * this is made borderline impossible by the fact that the size of the state "on the stack" is a block, and has to be expanded back to the appropriate size of the value being swapped, so in practice this would have to effectively pattern-match on the exact shape of the block-wise swapping algorithm, as a roundabout way of "patching `core::ptr` on the fly"
* (**this PR**) disable the block-wise swap optimization altogether when `#[cfg(target_arch = "spirv")`
  * I've tested it and it does in fact allow compiling `for i in 0..n` loops, which was my primary motivation
  * main downside IMO is the fact that `core` now acknowledges an out-of-tree backend
    * as a counterpoint, any attempt to compile Rust to SPIR-V would run into this problem, one way or another
* only enable the block-wise swap optimization on targets where it's been empirically proven to be an improvement
  * would avoid any surprises in terms of potentially-broken/inefficient codegen, in general
  * however, it may be universally applicable (thanks to caches), even if the optimal block size could differ
* move low-level swapping into an intrinsic, where the backend can choose any optimization approach it wants
  * this also has an impact on MIR optimizations (cc ``@rust-lang/wg-mir-opt)`` - which currently cannot hope to make sense of e.g. `Option::take` despite it being effectively `_0 = *_1;` `*_1 = None;` `return;`
  * long-term this is my preferred approach, and I can start working on it if that's desired, but I wanted to confirm that this swapping optimization is the final blocker for [Rust-GPU](https://github.com/EmbarkStudios/rust-gpu/) supporting e.g. range `for` loops

r? ``@nagisa`` cc ``@rust-lang/libs``
…llaumeGomez

rustdoc: Separate filter-empty-string out into its own function
…ions, r=Mark-Simulacrum

Tests: Remove redundant `ignore-tidy-linelength` annotations

This is step 2 towards fixing rust-lang#77548.

In the codegen and codegen-units test suites, the `//` comment markers
were kept in order not to affect any source locations. This is because
these tests cannot be automatically `--bless`ed.
ptr::addr_of documentation improvements

While writing rust-lang/reference#1001 I figured I could also improve the docs here a bit.
Remove attribute `#[link_args]`

Closes rust-lang#29596

The attribute could always be replaced with `-C link-arg`, but cargo didn't provide a reasonable way to pass such flags to rustc.
Now cargo supports `cargo:rustc-link-arg*` directives in build scripts (https://doc.rust-lang.org/cargo/reference/unstable.html#extra-link-arg), so this attribute can be removed.
Allow clobbering unsupported registers in asm!

Previously registers could only be marked as clobbered if the target feature for that register was enabled. This restriction is now removed.

cc rust-lang#81092

r? ``@nagisa``
@rustbot rustbot added the rollup A PR which is a rollup label Apr 4, 2021
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit f8709ec has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 4, 2021
@bors
Copy link
Contributor

bors commented Apr 5, 2021

⌛ Testing commit f8709ec with merge 015d2bc...

@bors
Copy link
Contributor

bors commented Apr 5, 2021

☀️ Test successful - checks-actions
Approved by: Dylan-DPC
Pushing 015d2bc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 5, 2021
@bors bors merged commit 015d2bc into rust-lang:master Apr 5, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup 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.