[experimental] Rvalue::InitBox prototype by nbdd0121 · Pull Request #88700 · rust-lang/rust

nbdd0121 · 2021-09-06T17:37:07Z

r? @ghost

Amanieu · 2021-09-06T17:42:32Z

@bors try @rust-timer queue

rust-timer · 2021-09-06T17:42:33Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-09-06T17:42:40Z

⌛ Trying commit 1dbf96965414a4778e996dc03952b9d0c318e884 with merge b375876e20ce4a2eff0f9588c09aa4e4114f696a...

bors · 2021-09-06T19:01:28Z

💔 Test failed - checks-actions

nbdd0121 · 2021-09-06T19:03:27Z

Do I actually need to fix all tools before a perf run could be done 🤔

Amanieu · 2021-09-06T22:10:54Z

@bors try @rust-timer queue

rust-timer · 2021-09-06T22:10:55Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-09-06T22:11:02Z

⌛ Trying commit 50f13a3 with merge a7b586082fae51c336ba78f005a339965e537c85...

bors · 2021-09-06T23:46:12Z

☀️ Try build successful - checks-actions
Build commit: a7b586082fae51c336ba78f005a339965e537c85 (a7b586082fae51c336ba78f005a339965e537c85)

rust-timer · 2021-09-06T23:46:13Z

Queued a7b586082fae51c336ba78f005a339965e537c85 with parent 8ceea01, future comparison URL.

rust-timer · 2021-09-07T03:05:45Z

Finished benchmarking commit (a7b586082fae51c336ba78f005a339965e537c85): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

Moderate improvement in instruction counts (up to -0.7% on incr-unchanged builds of html5ever)
Large regression in instruction counts (up to 10.6% on incr-patched: println builds of regression-31157)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

rust-log-analyzer · 2021-09-07T15:55:12Z

The job mingw-check failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

 Checking cranelift-native v0.75.0 (https://github.com/bytecodealliance/wasmtime.git#5deda279)
Checking cranelift-frontend v0.75.0 (https://github.com/bytecodealliance/wasmtime.git#5deda279)
Checking cranelift-object v0.75.0 (https://github.com/bytecodealliance/wasmtime.git#5deda279)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0004]: non-exhaustive patterns: `InitBox(_, _)` not covered
|
|
459 | match to_place_and_rval.1 {
| ^^^^^^^^^^^^^^^^^^^ pattern `InitBox(_, _)` not covered
::: /checkout/compiler/rustc_middle/src/mir/mod.rs:2174:5
|
|
2174 | InitBox(Operand<'tcx>, Ty<'tcx>),
|
= help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
= note: the matched value is of type `rustc_middle::mir::Rvalue`

oli-obk · 2021-09-07T16:34:11Z

@bors try @rust-timer queue

rust-timer · 2021-09-07T16:34:14Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-09-07T16:34:19Z

⌛ Trying commit a9c6bfb with merge 29328d5b353b632549fa019564fbf8e9daf44703...

bors · 2021-09-07T17:56:04Z

☀️ Try build successful - checks-actions
Build commit: 29328d5b353b632549fa019564fbf8e9daf44703 (29328d5b353b632549fa019564fbf8e9daf44703)

rust-timer · 2021-09-07T17:56:06Z

Queued 29328d5b353b632549fa019564fbf8e9daf44703 with parent 73641cd, future comparison URL.

rust-timer · 2021-09-07T20:20:21Z

Finished benchmarking commit (29328d5b353b632549fa019564fbf8e9daf44703): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

Large improvement in instruction counts (up to -2.5% on full builds of deeply-nested-async)
Moderate regression in instruction counts (up to 1.9% on incr-patched: println builds of syn)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

Introduce NullOp::AlignOf This PR introduces `Rvalue::NullaryOp(NullOp::AlignOf, ty)`, which will be lowered from `align_of`, similar to `size_of` lowering to `Rvalue::NullaryOp(NullOp::SizeOf, ty)`. The changes are originally part of rust-lang#88700 but since it's not dependent on other changes and could have performance impact on its own, it's separated into its own PR.

nbdd0121 · 2021-09-16T22:16:58Z

Closed this PR as it's an experiment; #89030 is the ready version for merge.

Introduce NullOp::AlignOf This PR introduces `Rvalue::NullaryOp(NullOp::AlignOf, ty)`, which will be lowered from `align_of`, similar to `size_of` lowering to `Rvalue::NullaryOp(NullOp::SizeOf, ty)`. The changes are originally part of rust-lang#88700 but since it's not dependent on other changes and could have performance impact on its own, it's separated into its own PR.

Introduce `Rvalue::ShallowInitBox` Polished version of rust-lang#88700. Implements MCP rust-lang/compiler-team#460, and should allow rust-lang#43596 to go forward. In short, creating an empty box is split from a nullary-op `NullOp::Box` into two steps, first a call to `exchange_malloc`, then a `Rvalue::ShallowInitBox` which transmutes `*mut u8` to a shallow-initialized `Box<T>`. This allows the `exchange_malloc` call to unwind. Details can be found in the MCP. `NullOp::Box` is not yet removed, purely to make reverting easier in case anything goes wrong as the result of this PR. If revert is needed a reversion of "Use Rvalue::ShallowInitBox for box expression" commit followed by a test bless should be sufficient. Experiments in rust-lang#88700 showed a very slight compile-time perf regression due to (supposedly) slightly more time spent in LLVM. We could omit unwind edge generation (in non-`oom=panic` case) in box expression MIR construction to restore perf; but I don't think it's necessary since runtime perf isn't affected and perf difference is rather small.

Introduce NullOp::AlignOf This PR introduces `Rvalue::NullaryOp(NullOp::AlignOf, ty)`, which will be lowered from `align_of`, similar to `size_of` lowering to `Rvalue::NullaryOp(NullOp::SizeOf, ty)`. The changes are originally part of rust-lang#88700 but since it's not dependent on other changes and could have performance impact on its own, it's separated into its own PR.

Introduce `Rvalue::ShallowInitBox` Polished version of rust-lang#88700. Implements MCP rust-lang/compiler-team#460, and should allow rust-lang#43596 to go forward. In short, creating an empty box is split from a nullary-op `NullOp::Box` into two steps, first a call to `exchange_malloc`, then a `Rvalue::ShallowInitBox` which transmutes `*mut u8` to a shallow-initialized `Box<T>`. This allows the `exchange_malloc` call to unwind. Details can be found in the MCP. `NullOp::Box` is not yet removed, purely to make reverting easier in case anything goes wrong as the result of this PR. If revert is needed a reversion of "Use Rvalue::ShallowInitBox for box expression" commit followed by a test bless should be sufficient. Experiments in rust-lang#88700 showed a very slight compile-time perf regression due to (supposedly) slightly more time spent in LLVM. We could omit unwind edge generation (in non-`oom=panic` case) in box expression MIR construction to restore perf; but I don't think it's necessary since runtime perf isn't affected and perf difference is rather small.

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 6, 2021

This comment has been minimized.
Sign in to view

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 6, 2021

nbdd0121 added 2 commits September 6, 2021 21:58

Rvalue::InitBox prototype
c8a2f53

Fix clippy
50f13a3

nbdd0121 force-pushed the box2 branch from 1dbf969 to 50f13a3Compare September 6, 2021 22:09

This comment has been minimized.
Sign in to view

rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Sep 7, 2021

nbdd0121 added 2 commits September 7, 2021 16:48

Introduce NullOp::AlignOf
bfa210e

Switch to use exchange_malloc
a9c6bfb

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 7, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 7, 2021

Amanieu mentioned this pull request Sep 10, 2021
Tracking issue for oom=panic (RFC 2116) #43596
Closed
6 tasks

nbdd0121 mentioned this pull request Sep 10, 2021
Introduce NullOp::AlignOf #88839
Merged

nbdd0121 closed this Sep 16, 2021

nbdd0121 mentioned this pull request Sep 16, 2021
Introduce Rvalue::ShallowInitBox#89030
Merged

Uh oh!

Uh oh!

Conversation

nbdd0121 commented Sep 6, 2021

Uh oh!

Amanieu commented Sep 6, 2021

Uh oh!

rust-timer commented Sep 6, 2021

Uh oh!

bors commented Sep 6, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Sep 6, 2021

Uh oh!

nbdd0121 commented Sep 6, 2021

Uh oh!

Amanieu commented Sep 6, 2021

Uh oh!

rust-timer commented Sep 6, 2021

Uh oh!

bors commented Sep 6, 2021

Uh oh!

This comment has been minimized.

bors commented Sep 6, 2021

Uh oh!

rust-timer commented Sep 6, 2021

Uh oh!

rust-timer commented Sep 7, 2021

Uh oh!

rust-log-analyzer commented Sep 7, 2021

Uh oh!

oli-obk commented Sep 7, 2021

Uh oh!

rust-timer commented Sep 7, 2021

Uh oh!

bors commented Sep 7, 2021

Uh oh!

bors commented Sep 7, 2021

Uh oh!

rust-timer commented Sep 7, 2021

Uh oh!

rust-timer commented Sep 7, 2021

Uh oh!

nbdd0121 commented Sep 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants