From d51aec7781186ae0a7ac1073f672d08729d32f6a Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Sat, 13 Sep 2025 17:36:46 +0000 Subject: [PATCH 1/4] Add test. --- ...ap.DestinationPropagation.panic-abort.diff | 21 ++++++++ ...p.DestinationPropagation.panic-unwind.diff | 21 ++++++++ tests/mir-opt/dest-prop/aggregate.rs | 49 +++++++++++++++++++ ...ap.DestinationPropagation.panic-abort.diff | 22 +++++++++ ...p.DestinationPropagation.panic-unwind.diff | 22 +++++++++ 5 files changed, 135 insertions(+) create mode 100644 tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff create mode 100644 tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff create mode 100644 tests/mir-opt/dest-prop/aggregate.rs create mode 100644 tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff create mode 100644 tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff diff --git a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff new file mode 100644 index 0000000000000..876c871cddde0 --- /dev/null +++ b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff @@ -0,0 +1,21 @@ +- // MIR for `rewrap` before DestinationPropagation ++ // MIR for `rewrap` after DestinationPropagation + + fn rewrap() -> (u8,) { + let mut _0: (u8,); + let mut _1: (u8,); + let mut _2: (u8,); + + bb0: { +- (_1.0: u8) = const 0_u8; +- _0 = copy _1; +- _2 = (copy (_0.0: u8),); +- _0 = copy _2; ++ (_0.0: u8) = const 0_u8; ++ nop; ++ _0 = (copy (_0.0: u8),); ++ nop; + return; + } + } + diff --git a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff new file mode 100644 index 0000000000000..876c871cddde0 --- /dev/null +++ b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff @@ -0,0 +1,21 @@ +- // MIR for `rewrap` before DestinationPropagation ++ // MIR for `rewrap` after DestinationPropagation + + fn rewrap() -> (u8,) { + let mut _0: (u8,); + let mut _1: (u8,); + let mut _2: (u8,); + + bb0: { +- (_1.0: u8) = const 0_u8; +- _0 = copy _1; +- _2 = (copy (_0.0: u8),); +- _0 = copy _2; ++ (_0.0: u8) = const 0_u8; ++ nop; ++ _0 = (copy (_0.0: u8),); ++ nop; + return; + } + } + diff --git a/tests/mir-opt/dest-prop/aggregate.rs b/tests/mir-opt/dest-prop/aggregate.rs new file mode 100644 index 0000000000000..3c14d1f94f690 --- /dev/null +++ b/tests/mir-opt/dest-prop/aggregate.rs @@ -0,0 +1,49 @@ +//@ test-mir-pass: DestinationPropagation +// EMIT_MIR_FOR_EACH_PANIC_STRATEGY + +#![feature(custom_mir, core_intrinsics)] +#![allow(internal_features)] + +use std::intrinsics::mir::*; +use std::mem::MaybeUninit; + +fn dump_var(_: T) {} + +// EMIT_MIR aggregate.rewrap.DestinationPropagation.diff +#[custom_mir(dialect = "runtime")] +fn rewrap() -> (u8,) { + // CHECK-LABEL: fn rewrap( + // CHECK: (_0.0: u8) = const 0_u8; + // CHECK: _0 = (copy (_0.0: u8),); + mir! { + let _1: (u8,); + let _2: (u8,); + { + _1.0 = 0; + RET = _1; + _2 = (RET.0, ); + RET = _2; + Return() + } + } +} + +// EMIT_MIR aggregate.swap.DestinationPropagation.diff +#[custom_mir(dialect = "runtime")] +fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) { + // CHECK-LABEL: fn swap( + // CHECK: _0 = const + // CHECK: _0 = (copy (_0.1: {{.*}}), copy (_0.0: {{.*}})); + mir! { + let _1: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>); + let _2: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>); + let _3: (); + { + _1 = const { (MaybeUninit::new([0; 10]), MaybeUninit::new([1; 10])) }; + _2 = _1; + _1 = (_2.1, _2.0); + RET = _1; + Return() + } + } +} diff --git a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff new file mode 100644 index 0000000000000..53849573f134c --- /dev/null +++ b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff @@ -0,0 +1,22 @@ +- // MIR for `swap` before DestinationPropagation ++ // MIR for `swap` after DestinationPropagation + + fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) { + let mut _0: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _1: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _2: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _3: (); + + bb0: { +- _1 = const swap::{constant#6}; +- _2 = copy _1; +- _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); +- _0 = copy _1; ++ _0 = const swap::{constant#6}; ++ nop; ++ _0 = (copy (_0.1: std::mem::MaybeUninit<[u8; 10]>), copy (_0.0: std::mem::MaybeUninit<[u8; 10]>)); ++ nop; + return; + } + } + diff --git a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff new file mode 100644 index 0000000000000..53849573f134c --- /dev/null +++ b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff @@ -0,0 +1,22 @@ +- // MIR for `swap` before DestinationPropagation ++ // MIR for `swap` after DestinationPropagation + + fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) { + let mut _0: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _1: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _2: (std::mem::MaybeUninit<[u8; 10]>, std::mem::MaybeUninit<[u8; 10]>); + let mut _3: (); + + bb0: { +- _1 = const swap::{constant#6}; +- _2 = copy _1; +- _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); +- _0 = copy _1; ++ _0 = const swap::{constant#6}; ++ nop; ++ _0 = (copy (_0.1: std::mem::MaybeUninit<[u8; 10]>), copy (_0.0: std::mem::MaybeUninit<[u8; 10]>)); ++ nop; + return; + } + } + From aee7d703c58b68611785684bbb8e402c1471992b Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Sat, 13 Sep 2025 18:07:22 +0000 Subject: [PATCH 2/4] Mark reads in statements to avoid overlapping assingments. --- compiler/rustc_mir_transform/src/dest_prop.rs | 29 ++++++++++++------- ...ap.DestinationPropagation.panic-abort.diff | 6 ++-- ...p.DestinationPropagation.panic-unwind.diff | 6 ++-- tests/mir-opt/dest-prop/aggregate.rs | 6 ++-- ...ap.DestinationPropagation.panic-abort.diff | 4 +-- ...p.DestinationPropagation.panic-unwind.diff | 4 +-- 6 files changed, 31 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 9ba2d274691de..7e6f39881b8b8 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -567,13 +567,15 @@ fn save_as_intervals<'tcx>( // the written-to locals as live in the second half of the statement. // We also ensure that operands read by terminators conflict with writes by that terminator. // For instance a function call may read args after having written to the destination. - VisitPlacesWith(|place, ctxt| match DefUse::for_place(place, ctxt) { - DefUse::Def | DefUse::Use | DefUse::PartialWrite => { - if let Some(relevant) = relevant.shrink[place.local] { - values.insert(relevant, twostep); + VisitPlacesWith(|place: Place<'tcx>, ctxt| { + if let Some(relevant) = relevant.shrink[place.local] { + match DefUse::for_place(place, ctxt) { + DefUse::Def | DefUse::Use | DefUse::PartialWrite => { + values.insert(relevant, twostep); + } + DefUse::NonUse => {} } } - DefUse::NonUse => {} }) .visit_terminator(term, loc); @@ -590,13 +592,20 @@ fn save_as_intervals<'tcx>( append_at(&mut values, &state, twostep); // Ensure we have a non-zero live range even for dead stores. This is done by marking // all the written-to locals as live in the second half of the statement. - VisitPlacesWith(|place, ctxt| match DefUse::for_place(place, ctxt) { - DefUse::Def | DefUse::PartialWrite => { - if let Some(relevant) = relevant.shrink[place.local] { - values.insert(relevant, twostep); + let is_simple_assignment = + matches!(stmt.kind, StatementKind::Assign(box (_, Rvalue::Use(_)))); + VisitPlacesWith(|place: Place<'tcx>, ctxt| { + if let Some(relevant) = relevant.shrink[place.local] { + match DefUse::for_place(place, ctxt) { + DefUse::Def | DefUse::PartialWrite => { + values.insert(relevant, twostep); + } + DefUse::Use if !is_simple_assignment => { + values.insert(relevant, twostep); + } + DefUse::Use | DefUse::NonUse => {} } } - DefUse::Use | DefUse::NonUse => {} }) .visit_statement(stmt, loc); diff --git a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff index 876c871cddde0..e80660f176b7b 100644 --- a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff +++ b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-abort.diff @@ -9,12 +9,10 @@ bb0: { - (_1.0: u8) = const 0_u8; - _0 = copy _1; -- _2 = (copy (_0.0: u8),); -- _0 = copy _2; + (_0.0: u8) = const 0_u8; + nop; -+ _0 = (copy (_0.0: u8),); -+ nop; + _2 = (copy (_0.0: u8),); + _0 = copy _2; return; } } diff --git a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff index 876c871cddde0..e80660f176b7b 100644 --- a/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff +++ b/tests/mir-opt/dest-prop/aggregate.rewrap.DestinationPropagation.panic-unwind.diff @@ -9,12 +9,10 @@ bb0: { - (_1.0: u8) = const 0_u8; - _0 = copy _1; -- _2 = (copy (_0.0: u8),); -- _0 = copy _2; + (_0.0: u8) = const 0_u8; + nop; -+ _0 = (copy (_0.0: u8),); -+ nop; + _2 = (copy (_0.0: u8),); + _0 = copy _2; return; } } diff --git a/tests/mir-opt/dest-prop/aggregate.rs b/tests/mir-opt/dest-prop/aggregate.rs index 3c14d1f94f690..636852159eb4c 100644 --- a/tests/mir-opt/dest-prop/aggregate.rs +++ b/tests/mir-opt/dest-prop/aggregate.rs @@ -14,7 +14,8 @@ fn dump_var(_: T) {} fn rewrap() -> (u8,) { // CHECK-LABEL: fn rewrap( // CHECK: (_0.0: u8) = const 0_u8; - // CHECK: _0 = (copy (_0.0: u8),); + // CHECK: _2 = (copy (_0.0: u8),); + // CHECK: _0 = copy _2; mir! { let _1: (u8,); let _2: (u8,); @@ -33,7 +34,8 @@ fn rewrap() -> (u8,) { fn swap() -> (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>) { // CHECK-LABEL: fn swap( // CHECK: _0 = const - // CHECK: _0 = (copy (_0.1: {{.*}}), copy (_0.0: {{.*}})); + // CHECK: _2 = copy _0; + // CHECK: _0 = (copy (_2.1: {{.*}}), copy (_2.0: {{.*}})); mir! { let _1: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>); let _2: (MaybeUninit<[u8; 10]>, MaybeUninit<[u8; 10]>); diff --git a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff index 53849573f134c..3aaad3aaf6920 100644 --- a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff +++ b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-abort.diff @@ -13,8 +13,8 @@ - _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); - _0 = copy _1; + _0 = const swap::{constant#6}; -+ nop; -+ _0 = (copy (_0.1: std::mem::MaybeUninit<[u8; 10]>), copy (_0.0: std::mem::MaybeUninit<[u8; 10]>)); ++ _2 = copy _0; ++ _0 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); + nop; return; } diff --git a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff index 53849573f134c..3aaad3aaf6920 100644 --- a/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff +++ b/tests/mir-opt/dest-prop/aggregate.swap.DestinationPropagation.panic-unwind.diff @@ -13,8 +13,8 @@ - _1 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); - _0 = copy _1; + _0 = const swap::{constant#6}; -+ nop; -+ _0 = (copy (_0.1: std::mem::MaybeUninit<[u8; 10]>), copy (_0.0: std::mem::MaybeUninit<[u8; 10]>)); ++ _2 = copy _0; ++ _0 = (copy (_2.1: std::mem::MaybeUninit<[u8; 10]>), copy (_2.0: std::mem::MaybeUninit<[u8; 10]>)); + nop; return; } From 8811344f22e0c015aaa45367b6462e3ecdcb22b5 Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Sun, 14 Sep 2025 13:04:53 +0000 Subject: [PATCH 3/4] Elaborate comment. --- compiler/rustc_mir_transform/src/dest_prop.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 7e6f39881b8b8..b9ab0a9feee65 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -590,8 +590,12 @@ fn save_as_intervals<'tcx>( twostep = TwoStepIndex::from_u32(twostep.as_u32() + 1); debug_assert_eq!(twostep, two_step_loc(loc, Effect::After)); append_at(&mut values, &state, twostep); - // Ensure we have a non-zero live range even for dead stores. This is done by marking - // all the written-to locals as live in the second half of the statement. + // Like terminators, ensure we have a non-zero live range even for dead stores. + // Some rvalues interleave reads and writes, for instance `Rvalue::Aggregate`, see + // https://github.com/rust-lang/rust/issues/146383. By precaution, treat statements + // as behaving so by default. + // We make an exception for simple assignments `_a.stuff = {copy|move} _b.stuff`, + // as marking `_b` live here would prevent unification. let is_simple_assignment = matches!(stmt.kind, StatementKind::Assign(box (_, Rvalue::Use(_)))); VisitPlacesWith(|place: Place<'tcx>, ctxt| { From d58061e6131f6141985faadb6794af540f81b1db Mon Sep 17 00:00:00 2001 From: Camille Gillot Date: Tue, 16 Sep 2025 01:22:10 +0000 Subject: [PATCH 4/4] Restrict simple assignment condition. --- compiler/rustc_mir_transform/src/dest_prop.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index b9ab0a9feee65..c57483a681140 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -596,8 +596,14 @@ fn save_as_intervals<'tcx>( // as behaving so by default. // We make an exception for simple assignments `_a.stuff = {copy|move} _b.stuff`, // as marking `_b` live here would prevent unification. - let is_simple_assignment = - matches!(stmt.kind, StatementKind::Assign(box (_, Rvalue::Use(_)))); + let is_simple_assignment = match stmt.kind { + StatementKind::Assign(box ( + lhs, + Rvalue::CopyForDeref(rhs) + | Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)), + )) => lhs.projection == rhs.projection, + _ => false, + }; VisitPlacesWith(|place: Place<'tcx>, ctxt| { if let Some(relevant) = relevant.shrink[place.local] { match DefUse::for_place(place, ctxt) {