-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Conditionally fuse small constant constant integer switches when lowering slice patterns #136417
Draft
xacrimon
wants to merge
12
commits into
rust-lang:master
Choose a base branch
from
xacrimon:merge-small-prim-switch
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Feb 2, 2025
25 tasks
@rustbot label +S-waiting-on-author -S-waiting-on-review |
rustbot
added
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Feb 2, 2025
This comment has been minimized.
This comment has been minimized.
The job Click to see the possible cause of the failure (guessed by this bot)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partial solution significantly improving #110870 but not a "perfect for every case" solution. Somewhat spiritual successor to #121540.
I recently hacked together a fully functional PoC of what was attempted in #121540 using a hidden
const impl Trait
lang-item shoved into a private core module, which allowedPartialEq
like behaviour and easy lowering ofTestKind::Eq
into efficient comparisons with very simple MIR.However I quickly discovered there was in fact more to the story than this. Unfortuantely we can't just use that hack or wait until we get
const impl Trait
onPartialEq
. Simply detecting constant sequences and lowering them carelessly intoTestKind::Eq
and through some trait yielded some very good improvements in very specific patterns but often significantly worsened codegen for "common" patterns. A typical example is a match of a&[u8]
against a decent set of branches, each having some OR patterns on top of bytestring literals, say you're converting an error string from C into an enum for example. Carelessly lowering these into slice comparisons eliminated opportunities for "match decision tree magic" and in my testing most commonly degenerated into a long sequence of one-by-one slice equality checks, then I might as well use anif .. else
chain...Good match lowering is highly conditional on being able to pick kinds of tests in an order that gives it as much information as possible for as little test cost as possible, and being able to extract meaningful info from said test results. As a result, something slightly more clever is probably going to be needed to provide consistently decent codegen. Whatever exactly that is, it probably involves deciding between slice tests, indexed primitive tests and fused constant tests dynamically while lowering.
This PR is the first in probably a larger long-running project for me focused on making improvements to this. In particular some way to more intelligently pick tests, give the test picker ability to reason about sequences of patterns and additional kinds of tests that can be picked for them, like slice equality.
Since we don't have
const impl Trait
for PartialEq yet, and anything related is quite a ways out, this PR attempts to cut some low hanging fruit that is easy to do now, will be necessary later anyhow (to properly utilize slice comparisons) and seems to improve both code size and performance substantially in match snippets I've looked at so far. It does this by doing two main new things:PatKind::Array
andPatKind::Slice
, allowing meaningful patterns to be identified in their elems/subpatterns and this translated into the generated match pair trees.u8
andu16
which can fuse many comparisons into one, without completely loosing the branching ability that makes slice comparisons hard to implement without strong regressions in typical patterns.Left to do before review on my end
r? @ghost