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 #78992

Closed
wants to merge 23 commits into from
Closed

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Nov 12, 2020

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

SkiFire13 and others added 23 commits November 7, 2020 22:20
This matches the capitalization of RiscV
The inliner looks if a sanitizer is enabled before considering
`no_sanitize` attribute as possible source of incompatibility.

The MIR inlining could happen in a crate with sanitizer disabled, but
code generation in a crate with sanitizer enabled, thus the attribute
would be incorrectly ignored.

To avoid the issue never inline functions with different `no_sanitize`
attributes.
The information about cold attribute is lost during inlining,
Avoid the issue by never inlining cold functions.
The callee body is already transformed; the condition is always false.
…ing, r=petrochenkov

Implement destructuring assignment for structs and slices

This is the second step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: rust-lang#71126). This PR is the second part of rust-lang#71156, which was split up to allow for easier review.

Note that the first PR (rust-lang#78748) is not merged yet, so it is included as the first commit in this one. I thought this would allow the review to start earlier because I have some time this weekend to respond to reviews. If `@petrochenkov` prefers to wait until the first PR is merged, I totally understand, of course.

This PR implements destructuring assignment for (tuple) structs and slices. In order to do this, the following *parser change* was necessary: struct expressions are not required to have a base expression, i.e. `Struct { a: 1, .. }` becomes legal (in order to act like a struct pattern).

Unfortunately, this PR slightly regresses the diagnostics implemented in rust-lang#77283. However, it is only a missing help message in `src/test/ui/issues/issue-77218.rs`. Other instances of this diagnostic are not affected. Since I don't exactly understand how this help message works and how to fix it yet, I was hoping it's OK to regress this temporarily and fix it in a follow-up PR.

Thanks to `@varkor` who helped with the implementation, particularly around the struct rest changes.

r? `@petrochenkov`
Improve BinaryHeap performance

By changing the condition in the loops from `child < end` to `child < end - 1` we're guaranteed that `right = child + 1 < end` and since finding the index of the biggest sibling can be done with an arithmetic operation we can remove a branch from the loop body. The case where there's no right child, i.e. `child == end - 1` is instead handled outside the loop, after it ends; note that if the loops ends early we can use `return` instead of `break` since the check `child == end - 1` will surely fail.

I've also removed a call to `<[T]>::swap` that was hiding a bound check that [wasn't being optimized by LLVM](https://godbolt.org/z/zrhdGM).

A quick benchmarks on my pc shows that the gains are pretty significant:

|name                 |before ns/iter  |after ns/iter  |diff ns/iter  |diff %    |speedup |
|---------------------|----------------|---------------|--------------|----------|--------|
|find_smallest_1000   | 352,565        | 260,098       |     -92,467  | -26.23%  | x 1.36 |
|from_vec             | 676,795        | 473,934       |    -202,861  | -29.97%  | x 1.43 |
|into_sorted_vec      | 469,511        | 304,275       |    -165,236  | -35.19%  | x 1.54 |
|pop                  | 483,198        | 373,778       |    -109,420  | -22.64%  | x 1.29 |

The other 2 benchmarks for `BinaryHeap` (`peek_mut_deref_mut` and `push`) weren't impacted and as such didn't show any significant change.
Add asm register information for SPIR-V

As discussed in [zulip](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Defining.20asm!.20for.20new.20architecture), we at [rust-gpu](https://github.com/EmbarkStudios/rust-gpu) would like to support `asm!` for our SPIR-V backend. However, we cannot do so purely without frontend support: [this match](https://github.com/rust-lang/rust/blob/d4ea0b3e46a0303d5802b632e88ba1ba84d9d16f/compiler/rustc_target/src/asm/mod.rs#L185) fails and so `asm!` is not supported ([error reported here](https://github.com/rust-lang/rust/blob/d4ea0b3e46a0303d5802b632e88ba1ba84d9d16f/compiler/rustc_ast_lowering/src/expr.rs#L1095)). To resolve this, we need to stub out register information for SPIR-V to support getting the `asm!` content all the way to [`AsmBuilderMethods::codegen_inline_asm`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_codegen_ssa/traits/trait.AsmBuilderMethods.html#tymethod.codegen_inline_asm), at which point the rust-gpu backend can do all the parsing and codegen that is needed.

This is a pretty weird PR - adding support for a backend that isn't in-tree feels pretty gross to me, but I don't see an easy way around this. `@Amanieu` said I should submit it anyway, so, here we are! Let me know if this needs to go through a more formal process (MCP?) and what I should do to help this along.

I based this off the [wasm asm PR](rust-lang#78684), which unfortunately this PR conflicts with that one quite a bit, sorry for any merge conflict pain :(

---

Some open questions:

- What do we call the register class? Some context, SPIR-V is an SSA-based IR, there are "instructions" that create IDs (referred to as `<id>` in the spec), which can be referenced by other instructions. So, `reg` isn't exactly accurate, they're SSA IDs, not re-assignable registers.
- What happens when a SPIR-V register gets to the LLVM backend? Right now it's a `bug!`, but should that be a `sess.fatal()`? I'm not sure if it's even possible to reach that point, maybe there's a check that prevents the `spirv` target from even reaching that codepath.
Never inline C variadics, cold functions, functions with incompatible attributes ...

... and fix generator inlining.

Closes rust-lang#67863.
Closes rust-lang#78859.
…aron1011

update rustfmt to v1.4.25

Contains changes from rust-lang/rustfmt#4507

r? `@Aaron1011`
Update cargo

5 commits in d5556aeb8405b1fe696adb6e297ad7a1f2989b62..8662ab427a8d6ad8047811cc4d78dbd20dd07699
2020-11-04 22:20:36 +0000 to 2020-11-12 03:47:53 +0000
- Check if rust-src contains a vendor dir, and patch it in (rust-lang/cargo#8834)
- Improve performance of almost fresh builds (rust-lang/cargo#8837)
- Use u32/64::to/from_le_bytes instead of bit fiddling (rust-lang/cargo#8847)
- Avoid constructing an anyhow::Error when not necessary (rust-lang/cargo#8844)
- Skip extracting .cargo-ok files from packages (rust-lang/cargo#8835)
extend min_const_generics param ty tests

Apparently we never tested for `u128` and `i128` before this, so I added a test for all types which are allowed.

r? `@varkor`
@rustbot rustbot added the rollup A PR which is a rollup label Nov 12, 2020
@m-ou-se
Copy link
Member Author

m-ou-se commented Nov 12, 2020

@bors r+ p=7 rollup=never

@bors
Copy link
Contributor

bors commented Nov 12, 2020

📌 Commit 3cc4599 has been approved by m-ou-se

@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 Nov 12, 2020
@bors
Copy link
Contributor

bors commented Nov 12, 2020

⌛ Testing commit 3cc4599 with merge 3a3dc38a8dae55e43a067552d093443d0e3f303a...

@bors
Copy link
Contributor

bors commented Nov 12, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2020
@m-ou-se m-ou-se closed this Nov 12, 2020
@m-ou-se m-ou-se deleted the rollup-wyj1pdc branch November 12, 2020 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants