-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[Experiment] Use stackful generator for typeck to workaround WithOptConstParam
#96843
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
Conversation
The job Click to see the possible cause of the failure (guessed by this bot)
|
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit a23c3e3 with merge 5eb5cc22ae0bb5652a266ea0287a880c64a2ab14... |
☀️ Try build successful - checks-actions |
Queued 5eb5cc22ae0bb5652a266ea0287a880c64a2ab14 with parent 83322c5, future comparison URL. |
Finished benchmarking commit (5eb5cc22ae0bb5652a266ea0287a880c64a2ab14): comparison url. Summary:
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 may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Footnotes |
@@ -29,3 +29,4 @@ rustc_trait_selection = { path = "../rustc_trait_selection" } | |||
rustc_ty_utils = { path = "../rustc_ty_utils" } | |||
rustc_lint = { path = "../rustc_lint" } | |||
rustc_serialize = { path = "../rustc_serialize" } | |||
stackful = { git = "https://github.com/nbdd0121/stackful.git", branch = "dev", default-features = false, features = ["nightly"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stack switching isn't possible on wasm. There is a proposal for it (https://github.com/WebAssembly/stack-switching), but AFAIK no implementations yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we don't support host tools on wasm anyway...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a couple of tiny patches rustc compiles just fine for wasm32-wasi and with https://github.com/bjorn3/browser_wasi_shim it even works in the browser. A linker are only necessary when you want to actually compile. For a check run you don't need a linker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this would make it significantly harder to make wasm32-wasi a tier 2 target with host tools if we ever want to do that in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we can always resort to running typeck multiple times... as long as unwinding is supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that asyncify can be used with wasm-opt: https://web.dev/asyncify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That page doesn't give any indication on how to use asyncify without a wrapper around the asyncified wasm module which tells when to unwind and when to rewind to the exact same point from which you unwound. Said wrapper is not possible with wasi, as the wasi runtime drives execution, not your own code. I also suspect asyncify only allows a single stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added wasm32 support to stackful. nbdd0121/stackful@f4d3693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the exact invocation of asyncify you need for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use wasm-opt --asyncify --pass-arg=asyncify-ignore-imports <INPUT> -o <OUTPUT>
. Asyncify has a conservative default so omitting the pass-arg should be fine too.
See https://rust-lang.zulipchat.com/#narrow/stream/260443-project-const-generics/topic/WithOptConstParam for related discussion.
r? @ghost