diff options
| -rw-r--r-- | compiler/rustc_mir_transform/src/gvn.rs | 45 | ||||
| -rw-r--r-- | tests/mir-opt/gvn_overlapping.fields.GVN.diff | 14 | ||||
| -rw-r--r-- | tests/mir-opt/gvn_overlapping.rs | 42 | ||||
| -rw-r--r-- | tests/mir-opt/gvn_overlapping.stable_projection.GVN.diff | 19 |
4 files changed, 90 insertions, 30 deletions
diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 9da6e034c3b..f867c130efb 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -896,18 +896,13 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { fn simplify_aggregate_to_copy( &mut self, - lhs: &Place<'tcx>, - rvalue: &mut Rvalue<'tcx>, - location: Location, - fields: &[VnIndex], + ty: Ty<'tcx>, variant_index: VariantIdx, + fields: &[VnIndex], ) -> Option<VnIndex> { - let Some(&first_field) = fields.first() else { - return None; - }; - let Value::Projection(copy_from_value, _) = *self.get(first_field) else { - return None; - }; + let Some(&first_field) = fields.first() else { return None }; + let Value::Projection(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) @@ -934,21 +929,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } } - // Allow introducing places with non-constant offsets, as those are still better than - // reconstructing an aggregate. - if self.ty(copy_from_local_value) == rvalue.ty(self.local_decls, self.tcx) - && let Some(place) = self.try_as_place(copy_from_local_value, location, true) - { - // Avoid creating `*a = copy (*b)`, as they might be aliases resulting in overlapping assignments. - // FIXME: This also avoids any kind of projection, not just derefs. We can add allowed projections. - if lhs.as_local().is_some() { - self.reused_locals.insert(place.local); - *rvalue = Rvalue::Use(Operand::Copy(place)); - } - return Some(copy_from_local_value); - } - - None + // Both must be variants of the same type. + if self.ty(copy_from_local_value) == ty { Some(copy_from_local_value) } else { None } } fn simplify_aggregate( @@ -1035,9 +1017,16 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { return Some(self.insert(ty, Value::Repeat(first, len))); } - if let Some(value) = - self.simplify_aggregate_to_copy(lhs, rvalue, location, &fields, variant_index) - { + if let Some(value) = self.simplify_aggregate_to_copy(ty, variant_index, &fields) { + // Allow introducing places with non-constant offsets, as those are still better than + // reconstructing an aggregate. But avoid creating `*a = copy (*b)`, as they might be + // aliases resulting in overlapping assignments. + let allow_complex_projection = + lhs.projection[..].iter().all(PlaceElem::is_stable_offset); + if let Some(place) = self.try_as_place(value, location, allow_complex_projection) { + self.reused_locals.insert(place.local); + *rvalue = Rvalue::Use(Operand::Copy(place)); + } return Some(value); } diff --git a/tests/mir-opt/gvn_overlapping.fields.GVN.diff b/tests/mir-opt/gvn_overlapping.fields.GVN.diff new file mode 100644 index 00000000000..0548f4e42e4 --- /dev/null +++ b/tests/mir-opt/gvn_overlapping.fields.GVN.diff @@ -0,0 +1,14 @@ +- // MIR for `fields` before GVN ++ // MIR for `fields` after GVN + + fn fields(_1: (Adt, Adt)) -> () { + let mut _0: (); + let mut _2: u32; + + bb0: { + _2 = copy (((_1.0: Adt) as variant#1).0: u32); + (_1.1: Adt) = Adt::Some(copy _2); + return; + } + } + diff --git a/tests/mir-opt/gvn_overlapping.rs b/tests/mir-opt/gvn_overlapping.rs index 99113445e68..f148a843561 100644 --- a/tests/mir-opt/gvn_overlapping.rs +++ b/tests/mir-opt/gvn_overlapping.rs @@ -2,11 +2,10 @@ #![feature(custom_mir, core_intrinsics)] -// Check that we do not create overlapping assignments. - use std::intrinsics::mir::*; // EMIT_MIR gvn_overlapping.overlapping.GVN.diff +/// Check that we do not create overlapping assignments. #[custom_mir(dialect = "runtime")] fn overlapping(_17: Adt) { // CHECK-LABEL: fn overlapping( @@ -26,6 +25,45 @@ fn overlapping(_17: Adt) { } } +// EMIT_MIR gvn_overlapping.stable_projection.GVN.diff +/// Check that we allow dereferences in the RHS if the LHS is a stable projection. +#[custom_mir(dialect = "runtime")] +fn stable_projection(_1: (Adt,)) { + // CHECK-LABEL: fn stable_projection( + // CHECK: let mut _2: *mut Adt; + // CHECK: let mut _4: &Adt; + // CHECK: (_1.0: Adt) = copy (*_4); + mir! { + let _2: *mut Adt; + let _3: u32; + let _4: &Adt; + { + _2 = core::ptr::addr_of_mut!(_1.0); + _4 = &(*_2); + _3 = Field(Variant((*_4), 1), 0); + _1.0 = Adt::Some(_3); + Return() + } + } +} + +// EMIT_MIR gvn_overlapping.fields.GVN.diff +/// Check that we do not create assignments between different fields of the same local. +#[custom_mir(dialect = "runtime")] +fn fields(_1: (Adt, Adt)) { + // CHECK-LABEL: fn fields( + // CHECK: _2 = copy (((_1.0: Adt) as variant#1).0: u32); + // CHECK-NEXT: (_1.1: Adt) = Adt::Some(copy _2); + mir! { + let _2: u32; + { + _2 = Field(Variant(_1.0, 1), 0); + _1.1 = Adt::Some(_2); + Return() + } + } +} + fn main() { overlapping(Adt::Some(0)); } diff --git a/tests/mir-opt/gvn_overlapping.stable_projection.GVN.diff b/tests/mir-opt/gvn_overlapping.stable_projection.GVN.diff new file mode 100644 index 00000000000..08835456591 --- /dev/null +++ b/tests/mir-opt/gvn_overlapping.stable_projection.GVN.diff @@ -0,0 +1,19 @@ +- // MIR for `stable_projection` before GVN ++ // MIR for `stable_projection` after GVN + + fn stable_projection(_1: (Adt,)) -> () { + let mut _0: (); + let mut _2: *mut Adt; + let mut _3: u32; + let mut _4: &Adt; + + bb0: { + _2 = &raw mut (_1.0: Adt); + _4 = &(*_2); + _3 = copy (((*_4) as variant#1).0: u32); +- (_1.0: Adt) = Adt::Some(copy _3); ++ (_1.0: Adt) = copy (*_4); + return; + } + } + |