Skip to content
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

See through aggregates in GVN #116270

Merged
merged 28 commits into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d284059
Do not remove unused definitions inside GVN.
cjgillot Sep 16, 2023
afd631c
Do not visit rvalues twice.
cjgillot Sep 23, 2023
38c86b0
Evaluate computed values to constants.
cjgillot Sep 19, 2023
db9bd9b
Do not intern too large aggregates.
cjgillot Sep 23, 2023
9389373
Do not transmute immediates to non-immediates.
cjgillot Sep 23, 2023
692e528
Simplify projections in GVN.
cjgillot Sep 16, 2023
48d2157
Simplify aggregate projections.
cjgillot Sep 16, 2023
f110f22
Simplify repeat expressions.
cjgillot Sep 16, 2023
23d4857
Do not compute actual aggregate type.
cjgillot May 3, 2023
80a5e85
Extract simplify_aggregate.
cjgillot May 21, 2023
dbf9ea3
Transform large arrays into Repeat expressions when possible.
cjgillot Sep 23, 2023
8162dc2
Do not intern GVN temps.
cjgillot Oct 3, 2023
ebc87bf
Directly intern values instead of copying them.
cjgillot Oct 7, 2023
ff6812c
Move provenance checks out of interning method.
cjgillot Oct 10, 2023
fbf0a0c
Explain why we check variant equality.
cjgillot Oct 11, 2023
59235a7
Fortify transmute check.
cjgillot Oct 12, 2023
e3538d1
Do not require absence of metadata.
cjgillot Oct 12, 2023
f08dc9b
Take an AllocId in intern_const_alloc_for_constprop.
cjgillot Oct 12, 2023
5e78b9c
Disambiguate non-deterministic constants.
cjgillot Oct 14, 2023
f6aa3ee
Complete comments.
cjgillot Oct 14, 2023
50559ce
Valtrees for primitive types are fine.
cjgillot Oct 14, 2023
ac0228d
FileCheck gvn.
cjgillot Oct 16, 2023
c4cc9ca
Do not merge fn pointer casts.
cjgillot Oct 21, 2023
d80eb3a
Verify that the alloc_id is Memory.
cjgillot Oct 22, 2023
eda1928
Typo.
cjgillot Oct 23, 2023
72f0e0e
Rename has_provance and tweaks comments.
cjgillot Oct 23, 2023
8561618
Directly check provenance from the AllocId.
cjgillot Oct 23, 2023
24be433
Apply suggestions from code review
cjgillot Oct 27, 2023
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
Prev Previous commit
Next Next commit
Simplify projections in GVN.
  • Loading branch information
cjgillot committed Oct 25, 2023
commit 692e5286479e16b3e9b47ab842b543985e1a6ceb
155 changes: 105 additions & 50 deletions compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeAndMut};
use rustc_span::DUMMY_SP;
use rustc_target::abi::{self, Abi, Size, VariantIdx, FIRST_VARIANT};
use std::borrow::Cow;

use crate::dataflow_const_prop::DummyMachine;
use crate::ssa::{AssignedValue, SsaLocals};
Expand Down Expand Up @@ -461,6 +462,87 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
Some(op)
}

fn project(
&mut self,
place: PlaceRef<'tcx>,
value: VnIndex,
proj: PlaceElem<'tcx>,
) -> Option<VnIndex> {
let proj = match proj {
ProjectionElem::Deref => {
let ty = place.ty(self.local_decls, self.tcx).ty;
if let Some(Mutability::Not) = ty.ref_mutability()
&& let Some(pointee_ty) = ty.builtin_deref(true)
&& pointee_ty.ty.is_freeze(self.tcx, self.param_env)
{
// An immutable borrow `_x` always points to the same value for the
// lifetime of the borrow, so we can merge all instances of `*_x`.
ProjectionElem::Deref
} else {
return None;
}
}
ProjectionElem::Downcast(name, index) => ProjectionElem::Downcast(name, index),
ProjectionElem::Field(f, ty) => ProjectionElem::Field(f, ty),
ProjectionElem::Index(idx) => {
let idx = self.locals[idx]?;
ProjectionElem::Index(idx)
}
ProjectionElem::ConstantIndex { offset, min_length, from_end } => {
ProjectionElem::ConstantIndex { offset, min_length, from_end }
}
ProjectionElem::Subslice { from, to, from_end } => {
ProjectionElem::Subslice { from, to, from_end }
}
ProjectionElem::OpaqueCast(ty) => ProjectionElem::OpaqueCast(ty),
ProjectionElem::Subtype(ty) => ProjectionElem::Subtype(ty),
};

Some(self.insert(Value::Projection(value, proj)))
}

/// Simplify the projection chain if we know better.
#[instrument(level = "trace", skip(self))]
fn simplify_place_projection(&mut self, place: &mut Place<'tcx>, location: Location) {
// If the projection is indirect, we treat the local as a value, so can replace it with
// another local.
if place.is_indirect()
&& let Some(base) = self.locals[place.local]
&& let Some(new_local) = self.try_as_local(base, location)
{
place.local = new_local;
self.reused_locals.insert(new_local);
}

let mut projection = Cow::Borrowed(&place.projection[..]);

for i in 0..projection.len() {
let elem = projection[i];
if let ProjectionElem::Index(idx) = elem
&& let Some(idx) = self.locals[idx]
{
if let Some(offset) = self.evaluated[idx].as_ref()
&& let Ok(offset) = self.ecx.read_target_usize(offset)
{
projection.to_mut()[i] = ProjectionElem::ConstantIndex {
offset,
min_length: offset + 1,
from_end: false,
};
} else if let Some(new_idx) = self.try_as_local(idx, location) {
projection.to_mut()[i] = ProjectionElem::Index(new_idx);
self.reused_locals.insert(new_idx);
}
}
}

if projection.is_owned() {
place.projection = self.tcx.mk_place_elems(&projection);
}

trace!(?place);
}

/// Represent the *value* which would be read from `place`, and point `place` to a preexisting
/// place with the same value (if that already exists).
#[instrument(level = "trace", skip(self), ret)]
Expand All @@ -469,6 +551,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
place: &mut Place<'tcx>,
location: Location,
) -> Option<VnIndex> {
self.simplify_place_projection(place, location);

// Invariant: `place` and `place_ref` point to the same value, even if they point to
// different memory locations.
let mut place_ref = place.as_ref();
Expand All @@ -483,53 +567,15 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
place_ref = PlaceRef { local, projection: &place.projection[index..] };
}

let proj = match proj {
ProjectionElem::Deref => {
let ty = Place::ty_from(
place.local,
&place.projection[..index],
self.local_decls,
self.tcx,
)
.ty;
if let Some(Mutability::Not) = ty.ref_mutability()
&& let Some(pointee_ty) = ty.builtin_deref(true)
&& pointee_ty.ty.is_freeze(self.tcx, self.param_env)
{
// An immutable borrow `_x` always points to the same value for the
// lifetime of the borrow, so we can merge all instances of `*_x`.
ProjectionElem::Deref
} else {
return None;
}
}
ProjectionElem::Field(f, ty) => ProjectionElem::Field(f, ty),
ProjectionElem::Index(idx) => {
let idx = self.locals[idx]?;
ProjectionElem::Index(idx)
}
ProjectionElem::ConstantIndex { offset, min_length, from_end } => {
ProjectionElem::ConstantIndex { offset, min_length, from_end }
}
ProjectionElem::Subslice { from, to, from_end } => {
ProjectionElem::Subslice { from, to, from_end }
}
ProjectionElem::Downcast(name, index) => ProjectionElem::Downcast(name, index),
ProjectionElem::OpaqueCast(ty) => ProjectionElem::OpaqueCast(ty),
ProjectionElem::Subtype(ty) => ProjectionElem::Subtype(ty),
};
value = self.insert(Value::Projection(value, proj));
let base = PlaceRef { local: place.local, projection: &place.projection[..index] };
value = self.project(base, value, proj)?;
}

if let Some(local) = self.try_as_local(value, location)
&& local != place.local
// in case we had no projection to begin with.
{
*place = local.into();
self.reused_locals.insert(local);
} else if place_ref.local != place.local
|| place_ref.projection.len() < place.projection.len()
{
if let Some(new_local) = self.try_as_local(value, location) {
place_ref = PlaceRef { local: new_local, projection: &[] };
}

if place_ref.local != place.local || place_ref.projection.len() < place.projection.len() {
// By the invariant on `place_ref`.
*place = place_ref.project_deeper(&[], self.tcx);
self.reused_locals.insert(place_ref.local);
Expand All @@ -545,7 +591,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
location: Location,
) -> Option<VnIndex> {
match *operand {
Operand::Constant(ref constant) => Some(self.insert(Value::Constant(constant.const_))),
Operand::Constant(ref mut constant) => {
let const_ = constant.const_.normalize(self.tcx, self.param_env);
Some(self.insert(Value::Constant(const_)))
}
Operand::Copy(ref mut place) | Operand::Move(ref mut place) => {
let value = self.simplify_place_value(place, location)?;
if let Some(const_) = self.try_as_constant(value) {
Expand Down Expand Up @@ -595,11 +644,13 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let ty = rvalue.ty(self.local_decls, self.tcx);
Value::Aggregate(ty, variant_index, fields?)
}
Rvalue::Ref(_, borrow_kind, place) => {
return self.new_pointer(place, AddressKind::Ref(borrow_kind));
Rvalue::Ref(_, borrow_kind, ref mut place) => {
self.simplify_place_projection(place, location);
return self.new_pointer(*place, AddressKind::Ref(borrow_kind));
}
Rvalue::AddressOf(mutbl, place) => {
return self.new_pointer(place, AddressKind::Address(mutbl));
Rvalue::AddressOf(mutbl, ref mut place) => {
self.simplify_place_projection(place, location);
return self.new_pointer(*place, AddressKind::Address(mutbl));
}

// Operations.
Expand Down Expand Up @@ -757,6 +808,10 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
self.tcx
}

fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, location: Location) {
self.simplify_place_projection(place, location);
}

fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
self.simplify_operand(operand, location);
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
#![feature(box_patterns)]
#![feature(cow_is_borrowed)]
#![feature(decl_macro)]
#![feature(is_sorted)]
#![feature(let_chains)]
Expand Down
18 changes: 12 additions & 6 deletions tests/mir-opt/gvn.dereferences.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
bb2: {
StorageDead(_7);
StorageDead(_6);
StorageLive(_8);
- StorageLive(_8);
+ nop;
_8 = &raw const (*_1);
StorageLive(_9);
StorageLive(_10);
Expand All @@ -92,7 +93,8 @@
bb4: {
StorageDead(_12);
StorageDead(_11);
StorageLive(_13);
- StorageLive(_13);
+ nop;
_13 = &raw mut (*_1);
StorageLive(_14);
StorageLive(_15);
Expand All @@ -112,7 +114,8 @@
bb6: {
StorageDead(_17);
StorageDead(_16);
StorageLive(_18);
- StorageLive(_18);
+ nop;
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
Expand Down Expand Up @@ -188,9 +191,12 @@
StorageDead(_32);
StorageDead(_31);
_0 = const ();
StorageDead(_18);
StorageDead(_13);
StorageDead(_8);
- StorageDead(_18);
- StorageDead(_13);
- StorageDead(_8);
+ nop;
+ nop;
+ nop;
return;
}
}
Expand Down
18 changes: 12 additions & 6 deletions tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
bb2: {
StorageDead(_7);
StorageDead(_6);
StorageLive(_8);
- StorageLive(_8);
+ nop;
_8 = &raw const (*_1);
StorageLive(_9);
StorageLive(_10);
Expand All @@ -92,7 +93,8 @@
bb4: {
StorageDead(_12);
StorageDead(_11);
StorageLive(_13);
- StorageLive(_13);
+ nop;
_13 = &raw mut (*_1);
StorageLive(_14);
StorageLive(_15);
Expand All @@ -112,7 +114,8 @@
bb6: {
StorageDead(_17);
StorageDead(_16);
StorageLive(_18);
- StorageLive(_18);
+ nop;
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
Expand Down Expand Up @@ -188,9 +191,12 @@
StorageDead(_32);
StorageDead(_31);
_0 = const ();
StorageDead(_18);
StorageDead(_13);
StorageDead(_8);
- StorageDead(_18);
- StorageDead(_13);
- StorageDead(_8);
+ nop;
+ nop;
+ nop;
return;
}
}
Expand Down
76 changes: 74 additions & 2 deletions tests/mir-opt/gvn.references.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@
let mut _15: *mut impl Sized;
let _16: ();
let mut _17: *mut impl Sized;
let _18: &mut impl Sized;
let mut _20: S<&mut impl Sized>;
let mut _21: &mut impl Sized;
let _22: ();
let mut _23: &impl Sized;
let _24: ();
let mut _25: &mut impl Sized;
let _26: ();
let mut _27: *const impl Sized;
let _28: ();
let mut _29: *mut impl Sized;
scope 1 {
debug r => _18;
let _19: &mut impl Sized;
scope 2 {
debug s => _19;
}
}

bb0: {
StorageLive(_2);
Expand Down Expand Up @@ -94,11 +112,65 @@
bb8: {
StorageDead(_17);
StorageDead(_16);
_0 = const ();
drop(_1) -> [return: bb9, unwind unreachable];
- StorageLive(_18);
+ nop;
_18 = &mut _1;
- StorageLive(_19);
+ nop;
StorageLive(_20);
StorageLive(_21);
- _21 = move _18;
- _20 = S::<&mut impl Sized>(move _21);
+ _21 = _18;
+ _20 = S::<&mut impl Sized>(_18);
StorageDead(_21);
_19 = move (_20.0: &mut impl Sized);
StorageDead(_20);
StorageLive(_22);
StorageLive(_23);
_23 = &(*_19);
_22 = opaque::<&impl Sized>(move _23) -> [return: bb9, unwind unreachable];
}

bb9: {
StorageDead(_23);
StorageDead(_22);
StorageLive(_24);
StorageLive(_25);
_25 = &mut (*_19);
_24 = opaque::<&mut impl Sized>(move _25) -> [return: bb10, unwind unreachable];
}

bb10: {
StorageDead(_25);
StorageDead(_24);
StorageLive(_26);
StorageLive(_27);
_27 = &raw const (*_19);
_26 = opaque::<*const impl Sized>(move _27) -> [return: bb11, unwind unreachable];
}

bb11: {
StorageDead(_27);
StorageDead(_26);
StorageLive(_28);
StorageLive(_29);
_29 = &raw mut (*_19);
_28 = opaque::<*mut impl Sized>(move _29) -> [return: bb12, unwind unreachable];
}

bb12: {
StorageDead(_29);
StorageDead(_28);
_0 = const ();
- StorageDead(_19);
- StorageDead(_18);
+ nop;
+ nop;
drop(_1) -> [return: bb13, unwind unreachable];
}

bb13: {
return;
}
}
Expand Down
Loading