Avoid generating overlapping assignments in DSE - #162998
Conversation
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
S-waiting-on-review
|
r? @adwinwhite rustbot has assigned @adwinwhite. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? mir-opt |
This comment has been minimized.
This comment has been minimized.
1433b2b to
4014254
Compare
| // it eligible to be moved-from in the argument list. That's backwards. | ||
| if !destination.is_indirect() { | ||
| state.insert(destination.local); | ||
| } |
There was a problem hiding this comment.
Can you describe in positive terms what it is trying to accomplish and why is it necessary?
Isn't the newly inserted condition below, i.e., place.local != destination.local, sufficient to avoid overlap?
There was a problem hiding this comment.
The objective is to rule out turning _1 = f(copy _1) into _1 = f(move _1).
I was musing for a while about fixing this by updating state or by adding another condition, and I decided that fixing it by adding another condition would be too clumsy. The simple check if the locals are equal rules out turning the copy into move for _1[0] = f(copy _1[1]). I simply forgot to remove that line from the diff before pushing.
There was a problem hiding this comment.
Can you rewrite this as a separate condition?
There are two orthogonal concerns, so a separate checks make more sense to me:
- Existing liveness condition: moving a local clobbers its value, so the local must be dead after.
- Missing no-overlap condition: destination cannot overlap with moved argument.
The simple check if the locals are equal rules out turning the copy into move for
_1[0] = f(copy _1[1]).
The behavior is the same here, isn't it?
4014254 to
c973b50
Compare
|
@bors r+ |
rust-bors
Bot
added
S-waiting-on-bors
Avoid generating overlapping assignments in DSE This is a fix for rust-lang#162997. Considering we also had rust-lang#155680, I really wonder if this pass should be using LivenessTransferFunction at all.
Avoid generating overlapping assignments in DSE This is a fix for rust-lang#162997. Considering we also had rust-lang#155680, I really wonder if this pass should be using LivenessTransferFunction at all.
Avoid generating overlapping assignments in DSE This is a fix for rust-lang#162997. Considering we also had rust-lang#155680, I really wonder if this pass should be using LivenessTransferFunction at all.
…uwer Rollup of 7 pull requests Successful merges: - #147876 (Check tainted_by_error in LateLint) - #162998 (Avoid generating overlapping assignments in DSE) - #163136 (library: prune allowed lints) - #163102 (remove unnecessary restriction with next-solver) - #163106 (emit the constant pattern note for raw identifier bindings) - #163118 (add `feature(field_projections)` fixme) - #163148 (Clean up diagnostic hashing)
6 participants
This is a fix for #162997.
Considering we also had #155680, I really wonder if this pass should be using LivenessTransferFunction at all.