Skip to content

Implement RFC 3943: MIR move elimination#157943

Open
Amanieu wants to merge 8 commits into
rust-lang:mainfrom
Amanieu:move-elimination
Open

Implement RFC 3943: MIR move elimination#157943
Amanieu wants to merge 8 commits into
rust-lang:mainfrom
Amanieu:move-elimination

Conversation

@Amanieu

@Amanieu Amanieu commented Jun 15, 2026

Copy link
Copy Markdown
Member

View all comments

This PR implements rust-lang/rfcs#3943: a MIR optimization pass that eliminates unnecessary copies. Since the new pass relies on the new MIR semantics from RFC 3943, it is gated behind the the -Z mir-move-elimination flag. Enabling this flag also disables the DestinationPropagation pass, which is completely superseded by this one.

There are 3 main parts to this optimization:

  • PreciseLiveness is an analysis that calculates, at a sub-statement granularity, the points in a function where a local requires storage to be allocated. This is more fine-grained than MaybeStorageLive, and takes borrows into account.
  • TailCopyToMove is a small pre-processing pass which turns copies into the return place just before a return into moves. This is necessary to allow the source of the copy to be unified with the return place.
  • MoveElimination is the main pass which optimizes away moves through place unification.

The RFC text and the top-level comments for the various passes have more details on the internals of the optimization.

r? @dianqk

cc @rust-lang/wg-mir-opt

@rustbot

rustbot commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 15, 2026
@rustbot

rustbot commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

dianqk is currently at their maximum review capacity.
They may take a while to respond.

@rustbot

This comment has been minimized.

@Amanieu Amanieu force-pushed the move-elimination branch from f12157e to ca2c2ac Compare June 15, 2026 22:47
@cjgillot cjgillot self-assigned this Jun 16, 2026

@cjgillot cjgillot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can part of the code, in particular the liveness analysis be shared with the existing DestinationPropagation? Or is this meant to supersede it completely?

View changes since this review

//
// This pass has 2 outputs: a set of kill points that mark the last use
// locations of locals and a per-block bitset indicating which locals are live
// on entry to that block.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a named struct for this?

tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
pass_name: Option<&'static str>,
) -> (Vec<(Local, Location)>, IndexVec<BasicBlock, DenseBitSet<Local>>) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BitMatrix<BasicBlock, Local>?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In PreciseLiveness::apply_block_start_effect we need to have a DenseBitSet<Local> to call .intersect on, which BitMatrix doesn't expose.

Comment on lines +387 to +490
// Notably this kills any dead results produced by a predecessor's terminator.
state.intersect(&live_on_entry[block]);

for local in state.iter() {
builder.gen_(local, points.entry_point(block), SplitPointEffect::Early);
}

for (statement_index, statement) in block_data.statements.iter().enumerate() {
let location = Location { block, statement_index };
let point = points.point_from_location(location);

// StorageDead always kills a local, even if it has been borrowed.
if let mir::StatementKind::StorageDead(local) = statement.kind {
builder.kill(local, point, SplitPointEffect::Late);
continue;
}

// Kill moved operands if the whole local was moved.
VisitPlacesWith(|place: Place<'tcx>, ctxt| {
if ctxt == PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) {
if let Some(local) = place.as_local() {
builder.kill(local, point, SplitPointEffect::Early);
}
}
})
.visit_statement(statement, location);

// Kill any locals which are no longer used after this statement.
for &(local, _) in kill_point_map[point] {
builder.kill(local, point, SplitPointEffect::Early);
}

// Gen destination places.
VisitPlacesWith(|place: Place<'tcx>, ctxt| match DefUse::for_place(place, ctxt) {
DefUse::Def | DefUse::PartialWrite => {
builder.gen_(place.local, point, SplitPointEffect::Late)
}
DefUse::Use | DefUse::NonUse => {}
})
.visit_statement(statement, location);

// Kill any dead destination places: they will only appear at
// the late point of the statement they are generated in, which is
// sufficient for determining overlap.
for &(local, _) in kill_point_map[point] {
builder.kill(local, point, SplitPointEffect::Late);
}
}

let location = Location { block, statement_index: block_data.statements.len() };
let point = points.point_from_location(location);
let terminator = block_data.terminator();

// Kill moved operands if the whole local was moved. Also kill dropped
// places if the entire local was dropped.
VisitPlacesWith(|place: Place<'tcx>, ctxt| {
if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Move)
| PlaceContext::MutatingUse(MutatingUseContext::Drop) = ctxt
{
if let Some(local) = place.as_local() {
builder.kill(local, point, SplitPointEffect::Early);
}
}
})
.visit_terminator(terminator, location);

// Kill any locals which are no longer used after this terminator.
for &(local, _) in kill_point_map[point] {
builder.kill(local, point, SplitPointEffect::Early);
}

// Gen destination places.
VisitPlacesWith(|place: Place<'tcx>, ctxt| match DefUse::for_place(place, ctxt) {
DefUse::Def | DefUse::PartialWrite => {
builder.gen_(place.local, point, SplitPointEffect::Late)
}
DefUse::Use | DefUse::NonUse => {}
})
.visit_terminator(terminator, location);

// Move arguments to a call are treated specially: the place that they
// represent is passed directly to the callee, which means that they are
// not allowed to alias any other move operand or the destination place.
// This is represented here by extending their live range to the late
// part, making it overlap with that of the destination place.
//
// Notably, this *doesn't* apply to TailCall.
if let mir::TerminatorKind::Call {
func: _,
args,
destination: _,
target: _,
unwind: _,
call_source: _,
fn_span: _,
} = &terminator.kind
{
for arg in args {
if let mir::Operand::Move(place) = arg.node {
builder.gen_(place.local, point, SplitPointEffect::Late);
builder.kill(place.local, point, SplitPointEffect::Late);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates a lot of code in impl Analysis for PreciseLiveness. Any way to deduplicate?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not easily: the two are sufficiently different that I'm not sure the code would be clearer if we tried to deduplicate. The PreciseLiveness analysis is simpler since it only needs to determine the liveness at the end of the statement. On the other hand matrix construction needs to determine the liveness both at the mid point of the instruction and after it, and record both in the matrix.

trace!("cannot unify {a:?} and {b:?} involving a rust-call tuple argument");
return None;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove a common projection tail from a and b before this match? Allowing to merge _1.foo.bar with _2.blah.foo.bar?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but that's not really useful in practice if you consider the kind of patterns that trigger this optimization. Specifically, in order for the lifetimes to not overlap the RHS of an assignment needs to end its lifetime at the assignment. This only happens for a move of a whole local (no projections) or for non-borrowed locals where this is its last use.

fn visit_aggregate_assign(
&mut self,
dest: Place<'tcx>,
project_field: impl Fn(TyCtxt<'tcx>, Place<'tcx>, FieldIdx, Ty<'tcx>) -> Place<'tcx>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the callback return a &[PlaceElem<'tcx>] and call project_deeper here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning a slice is awkward because there's no good lifetime to give it.

Comment on lines +91 to +95
let Some(next) = next else {
break;
};
state.block = next;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, the loop and this special case is meant to avoid cloning the state.used_after. What about just calling stack.push(TailState { block: next, state: state.used_after }), and letting the while let loop handle everything?

// Under the local lifetime semantics from RFC 3943, `StorageLive` does not allocate,
// and `StorageDead` has no effect if the local was already freed by a move. These
// markers therefore do not affect whether a copy can be treated as a final use.
StatementKind::StorageLive(_) | StatementKind::StorageDead(_) | StatementKind::Nop => {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't StorageDead(l) perform a used_after.remove(l)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a backwards analysis where used_after tracks any local that is used later on the current path. We only ever add to used_after as we walk backwards through the function, and never remove from it.

Comment on lines +148 to +150
if !process_rvalue(&mut assign.1, used_after) {
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !process_rvalue(&mut assign.1, used_after) {
return false;
}
return process_rvalue(&mut assign.1, used_after);

?

block: BasicBlock,
used_after: &mut DenseBitSet<Local>,
borrowed: &DenseBitSet<Local>,
) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to return a Result or ControlFlow here? Just a bool is a bit obscure.

Comment on lines +127 to +128
StatementKind::Assign(assign) => {
let dest = assign.0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you untuple assign inside the match pattern?

@Amanieu

Amanieu commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

Can part of the code, in particular the liveness analysis be shared with the existing DestinationPropagation? Or is this meant to supersede it completely?

This optimization is meant to supersede DestinationPropagation entirely, which is why -Z mir-move-elimination disables DestinationPropagation. Also PreciseLiveness is based on the new MIR semantics in RFC 3943 which hasn't been accepted, which technically makes this MIR optimization unsound at the moment.

Separately, I think there may be value in having the coroutine pass re-use PreciseLiveness to determine the set of locals live across yields.

//!
//! A local live range starts at the late point of any statement or terminator
//! that writes to it without a `Deref` projection. It ends at the early point
//! of a `StorageDead`, a whole-local `Drop`, a whole-local move operand, or the

@tmiasko tmiasko Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment as on RFC: the special case for Drop has no justification. Similar for the actual implementation.

View changes since the review

Amanieu added 4 commits June 23, 2026 01:02
- Changed `iter_intervals` to return `RangeInclusive` instead of `Range`
- Added `clear_row`, `disjoint_rows` and `intersects_range` methods
@Amanieu Amanieu force-pushed the move-elimination branch from 6724dd7 to 974d4c0 Compare June 23, 2026 00:08
@rustbot

rustbot commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Amanieu added 3 commits June 23, 2026 01:16
This flag also has the effect of disabling DestinationPropagation, which
is already covered by move elimination.
@Amanieu Amanieu force-pushed the move-elimination branch from 974d4c0 to 34776e8 Compare June 23, 2026 00:16
@rust-log-analyzer

This comment has been minimized.

@Amanieu Amanieu force-pushed the move-elimination branch from 34776e8 to 454a459 Compare June 23, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants