-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Avoid accessing HIR from MIR passes #95487
Conversation
Some changes occurred in src/tools/clippy. cc @rust-lang/clippy |
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 433197aa622b98c66102db69c5470e927272ba0c with merge ad478adbe52aeca09eb203fb2e6b059054a66ef0... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
LLVM tried to reduce memory consumption: https://reviews.llvm.org/D111105 |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 3ab00b6099549cee9ae116158615d509c699987d with merge e1983f64e110a75b26ac533a88c0530b287f0512... |
What do you mean? Did you mean to post on another PR? |
Do you still need HIR when you are doing MIR passes? |
I would love to drop HIR once THIR is built. However, the way to go there obscure. We may access HIR directly for diagnostic purposes. We may also access it indirectly through more targeted queries that have not been computed yet. Because of the way the compiler and incremental compilation is structured, we don't really have a way to discover those accesses statically. I relied on manual investigation using |
We do already drop the entire TyCtxt after generating llvm ir is done and while optimization passes are running in background threads. As @cjgillot meantioned being more eager with dropping things is not that easy unfortunately. |
☀️ Try build successful - checks-actions |
Queued e1983f64e110a75b26ac533a88c0530b287f0512 with parent bb5c437, future comparison URL. |
Finished benchmarking commit (e1983f64e110a75b26ac533a88c0530b287f0512): comparison url. Summary: This benchmark run shows 6 relevant improvements 🎉 but 5 relevant regressions 😿 to instruction counts.
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 @bors rollup=never |
r? @oli-obk |
☔ The latest upstream changes (presumably #95667) made this pull request unmergeable. Please resolve the merge conflicts. |
This allows to avoid looking at HIR from borrowck.
@bors r+ |
📌 Commit bbacfcb has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (027a232): comparison url. Summary:
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
hir_owner_nodes
contains a lot of information, and the query result is typically dirty. This forces dependent queries to be re-executed needlessly.This PR refactors some accesses to HIR to go through more targeted queries that yield the same result.
Based on #95435 and #95436