MIR: split Operand::Consume into Copy and Move. #46142
Conversation
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
kennytm
added
the
S-waiting-on-review
src/librustc/mir/mod.rs
Outdated
| @@ -1274,7 +1274,8 @@ pub struct VisibilityScopeData { | |||
| /// being nested in one another. | |||
| #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable)] | |||
| pub enum Operand<'tcx> { | |||
| Consume(Lvalue<'tcx>), | |||
| Copy(Lvalue<'tcx>), | |||
There was a problem hiding this comment.
Can we leave some comments explaining what these mean, in terms of the expectations they provide? I think something like this:
- Copy: The value must be available for use afterwards. This implies that the type of the lvalue must be
Copy; this is true by construction during build, but checked by the MIR type checker. - Move: The value will not be used again. Safe for values of all types (modulo future developments towards
?Move). Enforced by borrow checker. Sometimes Copy is converted to Move to enable "last-use" optimizations.
| } | ||
| _ => {} |
There was a problem hiding this comment.
Personally, I'd rather this exhaustive (and perhaps move the if on the arm above inside the arm. But I rate the probability of overlooking this match after some future change to Operand as fairly low, so it's not that important.
|
FYI: If PR #46093 goes in first, I think it will conflict with this in a way that git won't notice. This line will need to become a Move: https://github.com/rust-lang/rust/pull/46093/files#diff-69bf2a484158ebf56322b7ffb03601cdR108 |
|
☔ The latest upstream changes (presumably #45879) made this pull request unmergeable. Please resolve the merge conflicts. |
kennytm
added
S-waiting-on-author
|
@bors r+ |
|
📌 Commit cd99774 has been approved by |
|
Looks like this might be real? [00:19:46] error[E0599]: no associated item named `Consume` found for type `rustc::mir::Operand<'_>` in the current scope
[00:19:46] --> /checkout/src/librustc_mir/transform/add_moves_for_packed_drops.rs:135:34
[00:19:46] |
[00:19:46] 135 | Rvalue::Use(Operand::Consume(location.clone())));
[00:19:46] | ^^^^^^^^^^^^^^^^ |
|
@bors r=nikomatsakis |
|
📌 Commit 56fb59c has been approved by |
|
☔ The latest upstream changes (presumably #46312) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r=nikomatsakis |
|
📌 Commit 919ed40 has been approved by |
MIR: split Operand::Consume into Copy and Move. By encoding the choice of leaving the source untouched (`Copy`) and invalidating it (`Move`) in MIR, we can express moves of copyable values and have MIR borrow-checking enforce them, *including* ownership transfer of stack locals in calls (when the ABI passes by indirection). Optimizations could turn a "last-use" `Copy` into a `Move`, and the MIR borrow-checker, at least within the confines of safe code, could even do this when the underlying lvalue was borrowed. (However, that last part would be the first time lifetime inference affects code generation, AFAIK). Furthermore, as `Move`s invalidate borrows as well, for any local that is initialized only once, we can ignore borrows that happened before a `Move` and safely reuse/replace its memory storage. This will allow us to perform NRVO in the presence of short-lived borrows, unlike LLVM (currently), and even compute optimal `StorageLive...StorageDead` ranges instead of discarding them.
rustc_mir: implement an "lvalue reuse" optimization (aka destination propagation aka NRVO). Replaces a chain of moves, such as `a = ...; ... b = move a; ... f(&mut b) ... c = move b`, with the final destination, i.e. only `c = ...; ... f(&mut c); ...` remains (note that borrowing is allowed). **DO NOT MERGE** only for testing atm. Based on #46142.
|
☀️ Test successful - status-appveyor, status-travis |
By encoding the choice of leaving the source untouched (
Copy) and invalidating it (Move) in MIR, we can express moves of copyable values and have MIR borrow-checking enforce them, including ownership transfer of stack locals in calls (when the ABI passes by indirection).Optimizations could turn a "last-use"
Copyinto aMove, and the MIR borrow-checker, at least within the confines of safe code, could even do this when the underlying lvalue was borrowed.(However, that last part would be the first time lifetime inference affects code generation, AFAIK).
Furthermore, as
Moves invalidate borrows as well, for any local that is initialized only once, we can ignore borrows that happened before aMoveand safely reuse/replace its memory storage.This will allow us to perform NRVO in the presence of short-lived borrows, unlike LLVM (currently), and even compute optimal
StorageLive...StorageDeadranges instead of discarding them.