gvn: avoid creating overlapping assignments

Author: dianqk

compiler/rustc_mir_transform/src/gvn.rs

@@ -836,6 +836,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 #[instrument(level = "trace", skip(self), ret)]
 fnsimplify_rvalue(
 &mutself,
+lhs:&Place<'tcx>,
 rvalue:&mutRvalue<'tcx>,
 location:Location,
 ) -> Option<VnIndex>{
@@ -855,7 +856,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 Value::Repeat(op, amount)
 }
 Rvalue::NullaryOp(op, ty) => Value::NullaryOp(op, ty),
-Rvalue::Aggregate(..) => returnself.simplify_aggregate(rvalue, location),
+Rvalue::Aggregate(..) => returnself.simplify_aggregate(lhs,rvalue, location),
 Rvalue::Ref(_, borrow_kind,refmut place) => {
 self.simplify_place_projection(place, location);
 returnSome(self.new_pointer(*place,AddressKind::Ref(borrow_kind)));
@@ -943,6 +944,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 
 fnsimplify_aggregate_to_copy(
 &mutself,
+lhs:&Place<'tcx>,
 rvalue:&mutRvalue<'tcx>,
 location:Location,
 fields:&[VnIndex],
@@ -982,19 +984,24 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 
 // Allow introducing places with non-constant offsets, as those are still better than
 // reconstructing an aggregate.
-ifletSome(place) = self.try_as_place(copy_from_local_value, location,true){
-if rvalue.ty(self.local_decls,self.tcx) == place.ty(self.local_decls,self.tcx).ty{
+ifletSome(place) = self.try_as_place(copy_from_local_value, location,true)
+ && rvalue.ty(self.local_decls,self.tcx) == place.ty(self.local_decls,self.tcx).ty
+{
+// 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));
-returnSome(copy_from_local_value);
 }
+returnSome(copy_from_local_value);
 }
 
 None
 }
 
 fnsimplify_aggregate(
 &mutself,
+lhs:&Place<'tcx>,
 rvalue:&mutRvalue<'tcx>,
 location:Location,
 ) -> Option<VnIndex>{
@@ -1090,7 +1097,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 
 ifletAggregateTy::Def(_, _) = ty
  && letSome(value) =
-self.simplify_aggregate_to_copy(rvalue, location,&fields, variant_index)
+self.simplify_aggregate_to_copy(lhs,rvalue, location,&fields, variant_index)
 {
 returnSome(value);
 }
@@ -1765,7 +1772,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
 ifletStatementKind::Assign(box (refmut lhs,refmut rvalue)) = stmt.kind{
 self.simplify_place_projection(lhs, location);
 
-let value = self.simplify_rvalue(rvalue, location);
+let value = self.simplify_rvalue(lhs,rvalue, location);
 let value = ifletSome(local) = lhs.as_local()
  && self.ssa.is_ssa(local)
 // FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark

tests/mir-opt/gvn_overlapping.overlapping.GVN.diff

@@ -0,0 +1,18 @@
+- // MIR for `overlapping` before GVN
++ // MIR for `overlapping` after GVN
+
+ fn overlapping(_1: Adt) -> () {
+ let mut _0: ();
+ let mut _2: *mut Adt;
+ let mut _3: u32;
+ let mut _4: &Adt;
+
+ bb0: {
+ _2 = &raw mut _1;
+ _4 = &(*_2);
+ _3 = copy (((*_4) as variant#1).0: u32);
+ (*_2) = Adt::Some(copy _3);
+ return;
+ }
+ }
+

tests/mir-opt/gvn_overlapping.rs

@@ -0,0 +1,36 @@
+//@ test-mir-pass: GVN
+
+#![feature(custom_mir, core_intrinsics)]
+
+// Check that we do not create overlapping assignments.
+
+use std::intrinsics::mir::*;
+
+// EMIT_MIR gvn_overlapping.overlapping.GVN.diff
+#[custom_mir(dialect = "runtime")]
+fnoverlapping(_17:Adt){
+// CHECK-LABEL: fn overlapping(
+// CHECK: let mut [[PTR:.*]]: *mut Adt;
+// CHECK: (*[[PTR]]) = Adt::Some(copy {{.*}});
+mir!{
+let _33:*mutAdt;
+let _48:u32;
+let _73:&Adt;
+{
+ _33 = core::ptr::addr_of_mut!(_17);
+ _73 = &(*_33);
+ _48 = Field(Variant((*_73),1),0);
+(*_33) = Adt::Some(_48);
+Return()
+}
+}
+}
+
+fnmain(){
+overlapping(Adt::Some(0));
+}
+
+enumAdt{
+None,
+Some(u32),
+}