Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 87eb56c

Browse files
committed
Use a VnIndex in Address projection.
1 parent ca3d3c4 commit 87eb56c

File tree

7 files changed

+173
-79
lines changed

7 files changed

+173
-79
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 131 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ enum AddressKind {
163163
Address(RawPtrKind),
164164
}
165165

166+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
167+
enum AddressBase {
168+
/// This address is based on this local.
169+
Local(Local),
170+
/// This address is based on the deref of this pointer.
171+
Deref(VnIndex),
172+
}
173+
166174
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
167175
enum Value<'a, 'tcx> {
168176
// Root values.
@@ -191,7 +199,10 @@ enum Value<'a, 'tcx> {
191199
Repeat(VnIndex, ty::Const<'tcx>),
192200
/// The address of a place.
193201
Address {
194-
place: Place<'tcx>,
202+
base: AddressBase,
203+
// We do not use a plain `Place` as we want to be able to reason about indices.
204+
// This does not contain any `Deref` projection.
205+
projection: &'a [ProjectionElem<VnIndex, Ty<'tcx>>],
195206
kind: AddressKind,
196207
/// Give each borrow and pointer a different provenance, so we don't merge them.
197208
provenance: usize,
@@ -308,16 +319,38 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
308319

309320
/// Create a new `Value::Address` distinct from all the others.
310321
#[instrument(level = "trace", skip(self), ret)]
311-
fn new_pointer(&mut self, place: Place<'tcx>, kind: AddressKind) -> VnIndex {
322+
fn new_pointer(
323+
&mut self,
324+
place: Place<'tcx>,
325+
kind: AddressKind,
326+
location: Location,
327+
) -> Option<VnIndex> {
312328
let pty = place.ty(self.local_decls, self.tcx).ty;
313329
let ty = match kind {
314330
AddressKind::Ref(bk) => {
315331
Ty::new_ref(self.tcx, self.tcx.lifetimes.re_erased, pty, bk.to_mutbl_lossy())
316332
}
317333
AddressKind::Address(mutbl) => Ty::new_ptr(self.tcx, pty, mutbl.to_mutbl_lossy()),
318334
};
319-
let value = Value::Address { place, kind, provenance: self.next_opaque() };
320-
self.insert(ty, value)
335+
336+
let mut projection = place.projection.iter();
337+
let base = if place.is_indirect_first_projection() {
338+
let base = self.locals[place.local]?;
339+
// Skip the initial `Deref`.
340+
projection.next();
341+
AddressBase::Deref(base)
342+
} else {
343+
AddressBase::Local(place.local)
344+
};
345+
// Do not try evaluating inside `Index`, this has been done by `simplify_place_value`.
346+
let projection = self
347+
.arena
348+
.try_alloc_from_iter(
349+
projection.map(|proj| proj.try_map(|value| self.locals[value], |ty| ty).ok_or(())),
350+
)
351+
.ok()?;
352+
let value = Value::Address { base, projection, kind, provenance: self.next_opaque() };
353+
Some(self.insert(ty, value))
321354
}
322355

323356
#[inline]
@@ -458,14 +491,15 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
458491
let elem = elem.try_map(|_| None, |()| ty.ty)?;
459492
self.ecx.project(base, elem).discard_err()?
460493
}
461-
Address { place, kind: _, provenance: _ } => {
462-
if !place.is_indirect_first_projection() {
463-
return None;
464-
}
465-
let local = self.locals[place.local]?;
466-
let pointer = self.evaluated[local].as_ref()?;
494+
Address { base, projection, .. } => {
495+
debug_assert!(!projection.contains(&ProjectionElem::Deref));
496+
let pointer = match base {
497+
AddressBase::Deref(pointer) => self.evaluated[pointer].as_ref()?,
498+
// We have no stack to point to.
499+
AddressBase::Local(_) => return None,
500+
};
467501
let mut mplace = self.ecx.deref_pointer(pointer).discard_err()?;
468-
for elem in place.projection.iter().skip(1) {
502+
for elem in projection {
469503
// `Index` by constants should have been replaced by `ConstantIndex` by
470504
// `simplify_place_projection`.
471505
let elem = elem.try_map(|_| None, |ty| ty)?;
@@ -589,19 +623,51 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
589623
Some(op)
590624
}
591625

626+
/// Represent the *value* we obtain by dereferencing an `Address` value.
627+
#[instrument(level = "trace", skip(self), ret)]
628+
fn dereference_address(
629+
&mut self,
630+
base: AddressBase,
631+
projection: &[ProjectionElem<VnIndex, Ty<'tcx>>],
632+
) -> Option<VnIndex> {
633+
let (mut place_ty, mut value) = match base {
634+
// The base is a local, so we take the local's value and project from it.
635+
AddressBase::Local(local) => {
636+
let local = self.locals[local]?;
637+
let place_ty = PlaceTy::from_ty(self.ty(local));
638+
(place_ty, local)
639+
}
640+
// The base is a pointer's deref, so we introduce the implicit deref.
641+
AddressBase::Deref(reborrow) => {
642+
let place_ty = PlaceTy::from_ty(self.ty(reborrow));
643+
self.project(place_ty, reborrow, ProjectionElem::Deref)?
644+
}
645+
};
646+
for &proj in projection {
647+
(place_ty, value) = self.project(place_ty, value, proj)?;
648+
}
649+
Some(value)
650+
}
651+
652+
#[instrument(level = "trace", skip(self), ret)]
592653
fn project(
593654
&mut self,
594655
place_ty: PlaceTy<'tcx>,
595656
value: VnIndex,
596-
proj: PlaceElem<'tcx>,
597-
from_non_ssa_index: &mut bool,
657+
proj: ProjectionElem<VnIndex, Ty<'tcx>>,
598658
) -> Option<(PlaceTy<'tcx>, VnIndex)> {
599659
let projection_ty = place_ty.projection_ty(self.tcx, proj);
600660
let proj = match proj {
601661
ProjectionElem::Deref => {
602662
if let Some(Mutability::Not) = place_ty.ty.ref_mutability()
603663
&& projection_ty.ty.is_freeze(self.tcx, self.typing_env())
604664
{
665+
if let Value::Address { base, projection, .. } = self.get(value)
666+
&& let Some(value) = self.dereference_address(base, projection)
667+
{
668+
return Some((projection_ty, value));
669+
}
670+
605671
// An immutable borrow `_x` always points to the same value for the
606672
// lifetime of the borrow, so we can merge all instances of `*_x`.
607673
return Some((projection_ty, self.insert_deref(projection_ty.ty, value)));
@@ -638,10 +704,8 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
638704
}
639705
ProjectionElem::Index(idx) => {
640706
if let Value::Repeat(inner, _) = self.get(value) {
641-
*from_non_ssa_index |= self.locals[idx].is_none();
642707
return Some((projection_ty, inner));
643708
}
644-
let idx = self.locals[idx]?;
645709
ProjectionElem::Index(idx)
646710
}
647711
ProjectionElem::ConstantIndex { offset, min_length, from_end } => {
@@ -720,74 +784,71 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
720784
/// Represent the *value* which would be read from `place`, and point `place` to a preexisting
721785
/// place with the same value (if that already exists).
722786
#[instrument(level = "trace", skip(self), ret)]
723-
fn simplify_place_value(
787+
fn compute_place_value(
724788
&mut self,
725-
place: &mutPlace<'tcx>,
789+
place: Place<'tcx>,
726790
location: Location,
727-
) -> Option<VnIndex> {
728-
self.simplify_place_projection(place, location);
729-
791+
) -> Result<VnIndex, PlaceRef<'tcx>> {
730792
// Invariant: `place` and `place_ref` point to the same value, even if they point to
731793
// different memory locations.
732794
let mut place_ref = place.as_ref();
733795

734796
// Invariant: `value` holds the value up-to the `index`th projection excluded.
735-
let mut value = self.locals[place.local]?;
797+
let Some(mut value) = self.locals[place.local]else{returnErr(place_ref)};
736798
// Invariant: `value` has type `place_ty`, with optional downcast variant if needed.
737799
let mut place_ty = PlaceTy::from_ty(self.local_decls[place.local].ty);
738-
let mut from_non_ssa_index = false;
739800
for (index, proj) in place.projection.iter().enumerate() {
740-
if let Value::Projection(pointer, ProjectionElem::Deref) = self.get(value)
741-
&& let Value::Address { place: mut pointee, kind, .. } = self.get(pointer)
742-
&& let AddressKind::Ref(BorrowKind::Shared) = kind
743-
&& let Some(v) = self.simplify_place_value(&mut pointee, location)
744-
{
745-
value = v;
746-
// `pointee` holds a `Place`, so `ProjectionElem::Index` holds a `Local`.
747-
// That local is SSA, but we otherwise have no guarantee on that local's value at
748-
// the current location compared to its value where `pointee` was borrowed.
749-
if pointee.projection.iter().all(|elem| !matches!(elem, ProjectionElem::Index(_))) {
750-
place_ref =
751-
pointee.project_deeper(&place.projection[index..], self.tcx).as_ref();
752-
}
753-
}
754801
if let Some(local) = self.try_as_local(value, location) {
755802
// Both `local` and `Place { local: place.local, projection: projection[..index] }`
756803
// hold the same value. Therefore, following place holds the value in the original
757804
// `place`.
758805
place_ref = PlaceRef { local, projection: &place.projection[index..] };
759806
}
760807

761-
(place_ty, value) = self.project(place_ty, value, proj, &mut from_non_ssa_index)?;
808+
let Some(proj) = proj.try_map(|value| self.locals[value], |ty| ty) else {
809+
return Err(place_ref);
810+
};
811+
let Some(ty_and_value) = self.project(place_ty, value, proj) else {
812+
return Err(place_ref);
813+
};
814+
(place_ty, value) = ty_and_value;
762815
}
763816

764-
if let Value::Projection(pointer, ProjectionElem::Deref) = self.get(value)
765-
&& let Value::Address { place: mut pointee, kind, .. } = self.get(pointer)
766-
&& let AddressKind::Ref(BorrowKind::Shared) = kind
767-
&& let Some(v) = self.simplify_place_value(&mut pointee, location)
768-
{
769-
value = v;
770-
// `pointee` holds a `Place`, so `ProjectionElem::Index` holds a `Local`.
771-
// That local is SSA, but we otherwise have no guarantee on that local's value at
772-
// the current location compared to its value where `pointee` was borrowed.
773-
if pointee.projection.iter().all(|elem| !matches!(elem, ProjectionElem::Index(_))) {
774-
place_ref = pointee.project_deeper(&[], self.tcx).as_ref();
775-
}
776-
}
777-
if let Some(new_local) = self.try_as_local(value, location) {
778-
place_ref = PlaceRef { local: new_local, projection: &[] };
779-
} else if from_non_ssa_index {
780-
// If access to non-SSA locals is unavoidable, bail out.
781-
return None;
782-
}
817+
Ok(value)
818+
}
783819

784-
if place_ref.local != place.local || place_ref.projection.len() < place.projection.len() {
785-
// By the invariant on `place_ref`.
786-
*place = place_ref.project_deeper(&[], self.tcx);
787-
self.reused_locals.insert(place_ref.local);
788-
}
820+
/// Represent the *value* which would be read from `place`, and point `place` to a preexisting
821+
/// place with the same value (if that already exists).
822+
#[instrument(level = "trace", skip(self), ret)]
823+
fn simplify_place_value(
824+
&mut self,
825+
place: &mut Place<'tcx>,
826+
location: Location,
827+
) -> Option<VnIndex> {
828+
self.simplify_place_projection(place, location);
789829

790-
Some(value)
830+
match self.compute_place_value(*place, location) {
831+
Ok(value) => {
832+
if let Some(new_place) = self.try_as_place(value, location, true)
833+
&& (new_place.local != place.local
834+
|| new_place.projection.len() < place.projection.len())
835+
{
836+
*place = new_place;
837+
self.reused_locals.insert(new_place.local);
838+
}
839+
Some(value)
840+
}
841+
Err(place_ref) => {
842+
if place_ref.local != place.local
843+
|| place_ref.projection.len() < place.projection.len()
844+
{
845+
// By the invariant on `place_ref`.
846+
*place = place_ref.project_deeper(&[], self.tcx);
847+
self.reused_locals.insert(place_ref.local);
848+
}
849+
None
850+
}
851+
}
791852
}
792853

793854
#[instrument(level = "trace", skip(self), ret)]
@@ -834,11 +895,11 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
834895
Rvalue::Aggregate(..) => return self.simplify_aggregate(lhs, rvalue, location),
835896
Rvalue::Ref(_, borrow_kind, ref mut place) => {
836897
self.simplify_place_projection(place, location);
837-
return Some(self.new_pointer(*place, AddressKind::Ref(borrow_kind)));
898+
return self.new_pointer(*place, AddressKind::Ref(borrow_kind), location);
838899
}
839900
Rvalue::RawPtr(mutbl, ref mut place) => {
840901
self.simplify_place_projection(place, location);
841-
return Some(self.new_pointer(*place, AddressKind::Address(mutbl)));
902+
return self.new_pointer(*place, AddressKind::Address(mutbl), location);
842903
}
843904
Rvalue::WrapUnsafeBinder(ref mut op, _) => {
844905
let value = self.simplify_operand(op, location)?;
@@ -1080,12 +1141,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
10801141
}
10811142

10821143
// `&mut *p`, `&raw *p`, etc don't change metadata.
1083-
Value::Address { place, kind: _, provenance: _ }
1084-
if let PlaceRef { local, projection: [PlaceElem::Deref] } =
1085-
place.as_ref()
1086-
&& let Some(local_index) = self.locals[local] =>
1144+
Value::Address { base: AddressBase::Deref(reborrowed), projection, .. }
1145+
if projection.is_empty() =>
10871146
{
1088-
arg_index = local_index;
1147+
arg_index = reborrowed;
10891148
was_updated = true;
10901149
continue;
10911150
}
@@ -1431,9 +1490,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
14311490

14321491
// The length information is stored in the wide pointer.
14331492
// Reborrowing copies length information from one pointer to the other.
1434-
while let Value::Address { place:borrowed,.. } =self.get(inner)
1435-
&& let[PlaceElem::Deref] = borrowed.projection[..]
1436-
&& letSome(borrowed) = self.locals[borrowed.local]
1493+
while let Value::Address { base:AddressBase::Deref(borrowed), projection,.. } =
1494+
self.get(inner)
1495+
&& projection.is_empty()
14371496
{
14381497
inner = borrowed;
14391498
}

‎tests/mir-opt/gvn.dereference_indexing.GVN.panic-abort.diff‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
+ nop;
4444
StorageLive(_8);
4545
StorageLive(_9);
46-
_9 = copy (*_3);
46+
- _9 = copy (*_3);
47+
+ _9 = copy _1[_4];
4748
_8 = opaque::<u8>(move _9) -> [return: bb2, unwind unreachable];
4849
}
4950

‎tests/mir-opt/gvn.dereference_indexing.GVN.panic-unwind.diff‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
+ nop;
4444
StorageLive(_8);
4545
StorageLive(_9);
46-
_9 = copy (*_3);
46+
- _9 = copy (*_3);
47+
+ _9 = copy _1[_4];
4748
_8 = opaque::<u8>(move _9) -> [return: bb2, unwind continue];
4849
}
4950

‎tests/mir-opt/gvn.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,8 +1065,8 @@ fn dereference_indexing(array: [u8; 2], index: usize) {
10651065
&array[i]
10661066
};
10671067

1068-
// CHECK-NOT: [{{.*}}]
1069-
// CHECK: [[tmp:_.*]] = copy (*[[a]]);
1068+
// CHECK-NOT: StorageDead([[i]]);
1069+
// CHECK: [[tmp:_.*]] = copy _1[[[i]]];
10701070
// CHECK: opaque::<u8>(move [[tmp]])
10711071
opaque(*a);
10721072
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
- // MIR for `index_place` before GVN
2+
+ // MIR for `index_place` after GVN
3+
4+
fn index_place(_1: usize, _2: usize, _3: [i32; 5]) -> i32 {
5+
let mut _0: i32;
6+
let mut _4: &i32;
7+
8+
bb0: {
9+
_4 = &_3[_1];
10+
_1 = copy _2;
11+
_0 = copy (*_4);
12+
return;
13+
}
14+
}
15+

‎tests/mir-opt/gvn_repeat.repeat_local.GVN.diff‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
_4 = [copy _3; 5];
1111
_5 = &_4[_1];
1212
_1 = copy _2;
13-
- _0 = copy (*_5);
14-
+ _0 = copy _3;
13+
_0 = copy (*_5);
1514
return;
1615
}
1716
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /