Skip to content

gvn: avoid creating overlapping assignments #141218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
#[instrument(level = "trace", skip(self), ret)]
fn simplify_rvalue(
&mut self,
lhs: &Place<'tcx>,
rvalue: &mut Rvalue<'tcx>,
location: Location,
) -> Option<VnIndex> {
Expand All @@ -855,7 +856,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
Value::Repeat(op, amount)
}
Rvalue::NullaryOp(op, ty) => Value::NullaryOp(op, ty),
Rvalue::Aggregate(..) => return self.simplify_aggregate(rvalue, location),
Rvalue::Aggregate(..) => return self.simplify_aggregate(lhs, rvalue, location),
Rvalue::Ref(_, borrow_kind, ref mut place) => {
self.simplify_place_projection(place, location);
return Some(self.new_pointer(*place, AddressKind::Ref(borrow_kind)));
Expand Down Expand Up @@ -943,6 +944,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {

fn simplify_aggregate_to_copy(
&mut self,
lhs: &Place<'tcx>,
rvalue: &mut Rvalue<'tcx>,
location: Location,
fields: &[VnIndex],
Expand Down Expand Up @@ -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.
if let Some(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 {
if let Some(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));
return Some(copy_from_local_value);
}
return Some(copy_from_local_value);
}

None
}

fn simplify_aggregate(
&mut self,
lhs: &Place<'tcx>,
rvalue: &mut Rvalue<'tcx>,
location: Location,
) -> Option<VnIndex> {
Expand Down Expand Up @@ -1090,7 +1097,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {

if let AggregateTy::Def(_, _) = ty
&& let Some(value) =
self.simplify_aggregate_to_copy(rvalue, location, &fields, variant_index)
self.simplify_aggregate_to_copy(lhs, rvalue, location, &fields, variant_index)
{
return Some(value);
}
Expand Down Expand Up @@ -1765,7 +1772,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
if let StatementKind::Assign(box (ref mut lhs, ref mut 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 = if let Some(local) = lhs.as_local()
&& self.ssa.is_ssa(local)
// FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark
Expand Down
18 changes: 18 additions & 0 deletions tests/mir-opt/gvn_overlapping.overlapping.GVN.diff
Original file line number Diff line number Diff line change
@@ -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;
}
}

36 changes: 36 additions & 0 deletions tests/mir-opt/gvn_overlapping.rs
Original file line number Diff line number Diff line change
@@ -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")]
fn overlapping(_17: Adt) {
// CHECK-LABEL: fn overlapping(
// CHECK: let mut [[PTR:.*]]: *mut Adt;
// CHECK: (*[[PTR]]) = Adt::Some(copy {{.*}});
mir! {
let _33: *mut Adt;
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()
}
}
}

fn main() {
overlapping(Adt::Some(0));
}

enum Adt {
None,
Some(u32),
}
Loading