-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Replace discriminant_switch_effect
with more general version
#77242
Conversation
r? @lcnr (rust_highfive has picked a reviewer for you, use r? to override) |
6695e85
to
d83e406
Compare
...that allows arbitrary effects on each edge of a `SwitchInt` terminator.
d83e406
to
e65421d
Compare
e65421d
to
2364b58
Compare
This shouldn't really affect perf, although it does do slightly less work than the old version. Just in case. @bors try |
Awaiting bors try build completion |
⌛ Trying commit 2364b58 with merge b61b01669af3db2429eae891e9c9da925db8d359... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-actions |
@jyn514 AFAIK, we always pass |
@bors try |
⌛ Trying commit c0cd1b0 with merge 3191331f311bf46f945abd05d1e078654803bdc7... |
Splitting this into a separate lint sounds useful, I'll work on that. In the meantime you can add |
☀️ Try build successful - checks-actions, checks-azure |
Queued 3191331f311bf46f945abd05d1e078654803bdc7 with parent 623fb90, future comparison URL. |
I don't know enough about this. r? @jonas-schievink maybe |
Finished benchmarking try commit (3191331f311bf46f945abd05d1e078654803bdc7): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
@bors r+ |
📌 Commit c0cd1b0 has been approved by |
☀️ Test successful - checks-actions, checks-azure |
#68528 added a new edge-specific effect for
SwitchInt
terminators,discriminant_switch_effect
, to the dataflow framework. While this accomplished the short-term goal of making drop elaboration more precise, it wasn't really useful in other contexts: It only supportedSwitchInt
s on the discriminant of anenum
and did not allow effects to be applied along the "otherwise" branch. In const-propagation, for example, arbitrary edge-specific effects for the targets of aSwitchInt
can be used to remember the value amatch
scrutinee must have in each arm.This PR replaces
discriminant_switch_effect
with a more generalswitch_int_edge_effects
method. The new method has a slightly different interface from the other edge-specific effect methods (e.g.call_return_effect
). This divergence is explained in the new method's documentation, and reading the changes to the various dataflow impls as well asdirection.rs
should further clarify things. This PR should not change behavior.