From 0814cb7b27d91a07de049abe9c28029d2b0c581b Mon Sep 17 00:00:00 2001 From: dianqk Date: Mon, 27 Oct 2025 21:43:11 +0800 Subject: [PATCH] GVN: Avoid merging borrows from the dereferenced value --- compiler/rustc_mir_transform/src/gvn.rs | 55 +++++-- .../gvn_copy_aggregate.all_copy_2.GVN.diff | 16 +- tests/mir-opt/gvn_copy_aggregate.rs | 5 +- .../pre-codegen/deref_nested_borrows.rs | 8 + ...variant_a-{closure#0}.PreCodegen.after.mir | 150 +++++++++--------- ...variant_b-{closure#0}.PreCodegen.after.mir | 54 ++++--- 6 files changed, 167 insertions(+), 121 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 820998eed1005..20a8ee73db042 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -243,7 +243,13 @@ enum Value<'a, 'tcx> { // Extractions. /// This is the *value* obtained by projecting another value. - Projection(VnIndex, ProjectionElem), + Projection { + base: VnIndex, + elem: ProjectionElem, + /// Some values may be a borrow or pointer. + /// Give them a different provenance, so we don't merge them. + provenance: Option, + }, /// Discriminant of the given value. Discriminant(VnIndex), @@ -292,6 +298,7 @@ impl<'a, 'tcx> ValueSet<'a, 'tcx> { debug_assert!(match value { Value::Opaque(_) | Value::Address { .. } => true, Value::Constant { disambiguator, .. } => disambiguator.is_some(), + Value::Projection { provenance, .. } => provenance.is_some(), _ => false, }); @@ -542,9 +549,23 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { } fn insert_deref(&mut self, ty: Ty<'tcx>, value: VnIndex) -> VnIndex { - let value = self.insert(ty, Value::Projection(value, ProjectionElem::Deref)); - self.derefs.push(value); - value + let index = if ty.is_any_ptr() { + // Give each borrow and pointer a different provenance, so we don't merge them. + let index = self.insert_unique(ty, |provenance| Value::Projection { + base: value, + elem: ProjectionElem::Deref, + provenance: Some(provenance), + }); + self.evaluated[index] = Some(None); + index + } else { + self.insert( + ty, + Value::Projection { base: value, elem: ProjectionElem::Deref, provenance: None }, + ) + }; + self.derefs.push(index); + index } fn invalidate_derefs(&mut self) { @@ -648,7 +669,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { ImmTy::from_immediate(ptr_imm, ty).into() } - Projection(base, elem) => { + Projection { base, elem, .. } => { let base = self.eval_to_const(base)?; // `Index` by constants should have been replaced by `ConstantIndex` by // `simplify_place_projection`. @@ -827,8 +848,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { ProjectionElem::Field(f, _) => match self.get(value) { Value::Aggregate(_, fields) => return Some((projection_ty, fields[f.as_usize()])), Value::Union(active, field) if active == f => return Some((projection_ty, field)), - Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant)) - if let Value::Aggregate(written_variant, fields) = self.get(outer_value) + Value::Projection { + base, elem: ProjectionElem::Downcast(_, read_variant), .. + } if let Value::Aggregate(written_variant, fields) = self.get(base) // This pass is not aware of control-flow, so we do not know whether the // replacement we are doing is actually reachable. We could be in any arm of // ``` @@ -881,7 +903,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { ProjectionElem::UnwrapUnsafeBinder(_) => ProjectionElem::UnwrapUnsafeBinder(()), }; - let value = self.insert(projection_ty.ty, Value::Projection(value, proj)); + let value = self.insert( + projection_ty.ty, + Value::Projection { base: value, elem: proj, provenance: None }, + ); Some((projection_ty, value)) } @@ -1114,11 +1139,17 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { fields: &[VnIndex], ) -> Option { let Some(&first_field) = fields.first() else { return None }; - let Value::Projection(copy_from_value, _) = self.get(first_field) else { return None }; + let Value::Projection { base: copy_from_value, .. } = self.get(first_field) else { + return None; + }; // All fields must correspond one-to-one and come from the same aggregate value. if fields.iter().enumerate().any(|(index, &v)| { - if let Value::Projection(pointer, ProjectionElem::Field(from_index, _)) = self.get(v) + if let Value::Projection { + base: pointer, + elem: ProjectionElem::Field(from_index, _), + .. + } = self.get(v) && copy_from_value == pointer && from_index.index() == index { @@ -1130,7 +1161,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> { } let mut copy_from_local_value = copy_from_value; - if let Value::Projection(pointer, proj) = self.get(copy_from_value) + if let Value::Projection { base: pointer, elem: proj, .. } = self.get(copy_from_value) && let ProjectionElem::Downcast(_, read_variant) = proj { if variant_index == read_variant { @@ -1843,7 +1874,7 @@ impl<'tcx> VnState<'_, '_, 'tcx> { // If we are here, we failed to find a local, and we already have a `Deref`. // Trying to add projections will only result in an ill-formed place. return None; - } else if let Value::Projection(pointer, proj) = self.get(index) + } else if let Value::Projection { base: pointer, elem: proj, .. } = self.get(index) && (allow_complex_projection || proj.is_stable_offset()) && let Some(proj) = self.try_as_place_elem(self.ty(index), proj, loc) { diff --git a/tests/mir-opt/gvn_copy_aggregate.all_copy_2.GVN.diff b/tests/mir-opt/gvn_copy_aggregate.all_copy_2.GVN.diff index eed8cb7d62e70..2eeeff56cc778 100644 --- a/tests/mir-opt/gvn_copy_aggregate.all_copy_2.GVN.diff +++ b/tests/mir-opt/gvn_copy_aggregate.all_copy_2.GVN.diff @@ -29,17 +29,13 @@ _8 = copy (*_1); _2 = copy ((*_8).0: i32); - StorageLive(_3); -- _9 = copy (*_1); -- _3 = copy ((*_9).1: u64); -- StorageLive(_4); -- _10 = copy (*_1); -- _4 = copy ((*_10).2: [i8; 3]); + nop; -+ _9 = copy _8; -+ _3 = copy ((*_8).1: u64); + _9 = copy (*_1); + _3 = copy ((*_9).1: u64); +- StorageLive(_4); + nop; -+ _10 = copy _8; -+ _4 = copy ((*_8).2: [i8; 3]); + _10 = copy (*_1); + _4 = copy ((*_10).2: [i8; 3]); StorageLive(_5); _5 = copy _2; StorageLive(_6); @@ -47,7 +43,7 @@ StorageLive(_7); _7 = copy _4; - _0 = AllCopy { a: move _5, b: move _6, c: move _7 }; -+ _0 = copy (*_8); ++ _0 = AllCopy { a: copy _2, b: copy _3, c: copy _4 }; StorageDead(_7); StorageDead(_6); StorageDead(_5); diff --git a/tests/mir-opt/gvn_copy_aggregate.rs b/tests/mir-opt/gvn_copy_aggregate.rs index c9473025a15f2..dc75b3a736661 100644 --- a/tests/mir-opt/gvn_copy_aggregate.rs +++ b/tests/mir-opt/gvn_copy_aggregate.rs @@ -24,13 +24,12 @@ fn all_copy(v: &AllCopy) -> AllCopy { AllCopy { a, b, c } } +// FIXME: For nested references, we need knowing each borrows can be merged. // EMIT_MIR gvn_copy_aggregate.all_copy_2.GVN.diff fn all_copy_2(v: &&AllCopy) -> AllCopy { // CHECK-LABEL: fn all_copy_2( // CHECK: bb0: { - // CHECK-NOT: = AllCopy { {{.*}} }; - // CHECK: [[V1:_.*]] = copy (*_1); - // CHECK: _0 = copy (*[[V1]]); + // CHECK: _0 = AllCopy { {{.*}} }; let a = v.a; let b = v.b; let c = v.c; diff --git a/tests/mir-opt/pre-codegen/deref_nested_borrows.rs b/tests/mir-opt/pre-codegen/deref_nested_borrows.rs index 738cd981ae674..23ad0189a66de 100644 --- a/tests/mir-opt/pre-codegen/deref_nested_borrows.rs +++ b/tests/mir-opt/pre-codegen/deref_nested_borrows.rs @@ -2,6 +2,14 @@ fn src(x: &&u8) -> bool { // CHECK-LABEL: fn src( + // CHECK: debug y => [[Y:_.*]]; + // CHECK: bb0: + // CHECK: [[BORROW_u8:_.*]] = copy (*_1); + // CHECK: [[Y]] = copy (*[[BORROW_u8]]); + // CHECK: bb1: + // BORROW_u8 outside its lifetime in bb1. + // CHECK-NOT: copy (*[[BORROW_u8]]); + // CHECK: copy (*_1); // CHECK-NOT: _0 = const true; // CHECK: _0 = Eq({{.*}}, {{.*}}); // CHECK-NOT: _0 = const true; diff --git a/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir index 2cab88182962f..afac0d23115e6 100644 --- a/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir @@ -3,101 +3,107 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2: &&(usize, usize, usize, usize)) -> bool { let mut _0: bool; let mut _3: &(usize, usize, usize, usize); - let mut _6: bool; + let mut _4: &(usize, usize, usize, usize); + let mut _5: &(usize, usize, usize, usize); + let mut _6: &(usize, usize, usize, usize); let mut _9: bool; - let mut _10: bool; - let _13: &usize; - let _14: &usize; - let _15: &usize; + let mut _12: bool; + let mut _13: bool; let _16: &usize; - let mut _17: &&usize; - let mut _18: &&usize; - let mut _19: &&usize; + let _17: &usize; + let _18: &usize; + let _19: &usize; let mut _20: &&usize; let mut _21: &&usize; let mut _22: &&usize; let mut _23: &&usize; let mut _24: &&usize; + let mut _25: &&usize; + let mut _26: &&usize; + let mut _27: &&usize; scope 1 { - debug a => _13; - debug b => _14; - debug c => _15; - debug d => _16; + debug a => _16; + debug b => _17; + debug c => _18; + debug d => _19; scope 2 (inlined std::cmp::impls::::le) { - debug self => _17; - debug other => _18; + debug self => _20; + debug other => _21; scope 3 (inlined std::cmp::impls::::le) { - debug self => _13; - debug other => _15; - let mut _4: usize; - let mut _5: usize; + debug self => _16; + debug other => _18; + let mut _7: usize; + let mut _8: usize; } } scope 4 (inlined std::cmp::impls::::le) { - debug self => _19; - debug other => _20; + debug self => _22; + debug other => _23; scope 5 (inlined std::cmp::impls::::le) { - debug self => _16; - debug other => _14; - let mut _7: usize; - let mut _8: usize; + debug self => _19; + debug other => _17; + let mut _10: usize; + let mut _11: usize; } } scope 6 (inlined std::cmp::impls::::le) { - debug self => _21; - debug other => _22; + debug self => _24; + debug other => _25; scope 7 (inlined std::cmp::impls::::le) { - debug self => _15; - debug other => _13; + debug self => _18; + debug other => _16; } } scope 8 (inlined std::cmp::impls::::le) { - debug self => _23; - debug other => _24; + debug self => _26; + debug other => _27; scope 9 (inlined std::cmp::impls::::le) { - debug self => _14; - debug other => _16; - let mut _11: usize; - let mut _12: usize; + debug self => _17; + debug other => _19; + let mut _14: usize; + let mut _15: usize; } } } bb0: { _3 = copy (*_2); - // DBG: _13 = &((*_3).0: usize); - // DBG: _14 = &((*_3).1: usize); - // DBG: _15 = &((*_3).2: usize); - // DBG: _16 = &((*_3).3: usize); - StorageLive(_6); - // DBG: _17 = &_13; - // DBG: _18 = &?; - _4 = copy ((*_3).0: usize); - _5 = copy ((*_3).2: usize); - _6 = Le(copy _4, copy _5); - switchInt(move _6) -> [0: bb2, otherwise: bb1]; + // DBG: _16 = &((*_3).0: usize); + _4 = copy (*_2); + // DBG: _17 = &((*_4).1: usize); + _5 = copy (*_2); + // DBG: _18 = &((*_5).2: usize); + _6 = copy (*_2); + // DBG: _19 = &((*_6).3: usize); + StorageLive(_9); + // DBG: _20 = &_16; + // DBG: _21 = &?; + _7 = copy ((*_3).0: usize); + _8 = copy ((*_5).2: usize); + _9 = Le(copy _7, copy _8); + switchInt(move _9) -> [0: bb2, otherwise: bb1]; } bb1: { - StorageLive(_9); - // DBG: _19 = &_16; - // DBG: _20 = &?; - StorageLive(_7); - _7 = copy ((*_3).3: usize); - StorageLive(_8); - _8 = copy ((*_3).1: usize); - _9 = Le(move _7, move _8); - StorageDead(_8); - StorageDead(_7); - switchInt(move _9) -> [0: bb2, otherwise: bb6]; + StorageLive(_12); + // DBG: _22 = &_19; + // DBG: _23 = &?; + StorageLive(_10); + _10 = copy ((*_6).3: usize); + StorageLive(_11); + _11 = copy ((*_4).1: usize); + _12 = Le(move _10, move _11); + StorageDead(_11); + StorageDead(_10); + switchInt(move _12) -> [0: bb2, otherwise: bb6]; } bb2: { - StorageLive(_10); - // DBG: _21 = &_15; - // DBG: _22 = &?; - _10 = Le(copy _5, copy _4); - switchInt(move _10) -> [0: bb3, otherwise: bb4]; + StorageLive(_13); + // DBG: _24 = &_18; + // DBG: _25 = &?; + _13 = Le(copy _8, copy _7); + switchInt(move _13) -> [0: bb3, otherwise: bb4]; } bb3: { @@ -106,20 +112,20 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 } bb4: { - // DBG: _23 = &_14; - // DBG: _24 = &?; - StorageLive(_11); - _11 = copy ((*_3).1: usize); - StorageLive(_12); - _12 = copy ((*_3).3: usize); - _0 = Le(move _11, move _12); - StorageDead(_12); - StorageDead(_11); + // DBG: _26 = &_17; + // DBG: _27 = &?; + StorageLive(_14); + _14 = copy ((*_4).1: usize); + StorageLive(_15); + _15 = copy ((*_6).3: usize); + _0 = Le(move _14, move _15); + StorageDead(_15); + StorageDead(_14); goto -> bb5; } bb5: { - StorageDead(_10); + StorageDead(_13); goto -> bb7; } @@ -129,8 +135,8 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 } bb7: { + StorageDead(_12); StorageDead(_9); - StorageDead(_6); return; } } diff --git a/tests/mir-opt/pre-codegen/slice_filter.variant_b-{closure#0}.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/slice_filter.variant_b-{closure#0}.PreCodegen.after.mir index bc7a31d52199b..f93f7264dec20 100644 --- a/tests/mir-opt/pre-codegen/slice_filter.variant_b-{closure#0}.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/slice_filter.variant_b-{closure#0}.PreCodegen.after.mir @@ -4,40 +4,46 @@ fn variant_b::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:11:25: 11:41}, let mut _0: bool; let mut _3: &(usize, usize, usize, usize); let _4: usize; - let _5: usize; + let mut _5: &(usize, usize, usize, usize); let _6: usize; - let _7: usize; - let mut _8: bool; - let mut _9: bool; - let mut _10: bool; + let mut _7: &(usize, usize, usize, usize); + let _8: usize; + let mut _9: &(usize, usize, usize, usize); + let _10: usize; + let mut _11: bool; + let mut _12: bool; + let mut _13: bool; scope 1 { debug a => _4; - debug b => _5; - debug c => _6; - debug d => _7; + debug b => _6; + debug c => _8; + debug d => _10; } bb0: { _3 = copy (*_2); _4 = copy ((*_3).0: usize); - _5 = copy ((*_3).1: usize); - _6 = copy ((*_3).2: usize); - _7 = copy ((*_3).3: usize); - StorageLive(_8); - _8 = Le(copy _4, copy _6); - switchInt(move _8) -> [0: bb2, otherwise: bb1]; + _5 = copy (*_2); + _6 = copy ((*_5).1: usize); + _7 = copy (*_2); + _8 = copy ((*_7).2: usize); + _9 = copy (*_2); + _10 = copy ((*_9).3: usize); + StorageLive(_11); + _11 = Le(copy _4, copy _8); + switchInt(move _11) -> [0: bb2, otherwise: bb1]; } bb1: { - StorageLive(_9); - _9 = Le(copy _7, copy _5); - switchInt(move _9) -> [0: bb2, otherwise: bb6]; + StorageLive(_12); + _12 = Le(copy _10, copy _6); + switchInt(move _12) -> [0: bb2, otherwise: bb6]; } bb2: { - StorageLive(_10); - _10 = Le(copy _6, copy _4); - switchInt(move _10) -> [0: bb3, otherwise: bb4]; + StorageLive(_13); + _13 = Le(copy _8, copy _4); + switchInt(move _13) -> [0: bb3, otherwise: bb4]; } bb3: { @@ -46,12 +52,12 @@ fn variant_b::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:11:25: 11:41}, } bb4: { - _0 = Le(copy _5, copy _7); + _0 = Le(copy _6, copy _10); goto -> bb5; } bb5: { - StorageDead(_10); + StorageDead(_13); goto -> bb7; } @@ -61,8 +67,8 @@ fn variant_b::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:11:25: 11:41}, } bb7: { - StorageDead(_9); - StorageDead(_8); + StorageDead(_12); + StorageDead(_11); return; } }