Skip to content

Commit 48bb7ae

Browse files
cjgillotcjgillot
committed
Extend GVN to perform local value numbering.
1 parent 5046eb6 commit 48bb7ae

27 files changed

+376
-245
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 90 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ use rustc_const_eval::interpret::{
9595
ImmTy, Immediate, InterpCx, MemPlaceMeta, MemoryKind, OpTy, Projectable, Scalar,
9696
intern_const_alloc_for_constprop,
9797
};
98-
use rustc_data_structures::fx::{FxIndexSet, MutableValues};
98+
use rustc_data_structures::fx::{FxHashMap, FxIndexSet, MutableValues};
9999
use rustc_data_structures::graph::dominators::Dominators;
100100
use rustc_hir::def::DefKind;
101101
use rustc_index::bit_set::DenseBitSet;
@@ -235,8 +235,11 @@ struct VnState<'body, 'a, 'tcx> {
235235
/// Value stored in each local.
236236
locals: IndexVec<Local, Option<VnIndex>>,
237237
/// Locals that are assigned that value.
238-
// This vector does not hold all the values of `VnIndex` that we create.
239-
rev_locals: IndexVec<VnIndex, SmallVec<[Local; 1]>>,
238+
// This vector holds the locals that are SSA.
239+
rev_locals_ssa: IndexVec<VnIndex, SmallVec<[Local; 1]>>,
240+
// This map holds the locals that are not SSA. This map is cleared at the end of each block.
241+
// Therefore, we do not need a location, the local always appears before the current location.
242+
rev_locals_non_ssa: FxHashMap<VnIndex, SmallVec<[Local; 1]>>,
240243
values: FxIndexSet<(Value<'a, 'tcx>, Ty<'tcx>)>,
241244
/// Values evaluated as constants if possible.
242245
evaluated: IndexVec<VnIndex, Option<OpTy<'tcx>>>,
@@ -272,8 +275,9 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
272275
ecx: InterpCx::new(tcx, DUMMY_SP, typing_env, DummyMachine),
273276
local_decls,
274277
is_coroutine: body.coroutine.is_some(),
275-
locals: IndexVec::from_elem(None, local_decls),
276-
rev_locals: IndexVec::with_capacity(num_values),
278+
locals: IndexVec::from_elem(None, &body.local_decls),
279+
rev_locals_ssa: IndexVec::with_capacity(num_values),
280+
rev_locals_non_ssa: FxHashMap::default(),
277281
values: FxIndexSet::with_capacity_and_hasher(num_values, Default::default()),
278282
evaluated: IndexVec::with_capacity(num_values),
279283
next_opaque: 1,
@@ -298,7 +302,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
298302
let evaluated = self.eval_to_const(index);
299303
let _index = self.evaluated.push(evaluated);
300304
debug_assert_eq!(index, _index);
301-
let _index = self.rev_locals.push(SmallVec::new());
305+
let _index = self.rev_locals_ssa.push(Default::default());
302306
debug_assert_eq!(index, _index);
303307
}
304308
index
@@ -336,7 +340,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
336340

337341
let mut projection = place.projection.iter();
338342
let base = if place.is_indirect_first_projection() {
339-
let base = self.locals[place.local]?;
343+
let base = self.local(place.local);
340344
// Skip the initial `Deref`.
341345
projection.next();
342346
AddressBase::Deref(base)
@@ -347,7 +351,8 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
347351
let projection = self
348352
.arena
349353
.try_alloc_from_iter(
350-
projection.map(|proj| proj.try_map(|value| self.locals[value], |ty| ty).ok_or(())),
354+
projection
355+
.map(|proj| proj.try_map(|value| Some(self.local(value)), |ty| ty).ok_or(())),
351356
)
352357
.ok()?;
353358
let value = Value::Address { base, projection, kind, provenance: self.next_opaque() };
@@ -364,12 +369,49 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
364369
self.values.get_index(index.as_usize()).unwrap().1
365370
}
366371

367-
/// Record that `local` is assigned `value`. `local` must be SSA.
372+
/// Record that `local` is assigned `value`.
368373
#[instrument(level = "trace", skip(self))]
369374
fn assign(&mut self, local: Local, value: VnIndex) {
370-
debug_assert!(self.ssa.is_ssa(local));
371375
self.locals[local] = Some(value);
372-
self.rev_locals[value].push(local);
376+
if self.ssa.is_ssa(local) {
377+
self.rev_locals_ssa[value].push(local);
378+
} else {
379+
self.rev_locals_non_ssa.entry(value).or_default().push(local);
380+
}
381+
}
382+
383+
/// Return the value assigned to a local, or assign an opaque value and return it.
384+
#[instrument(level = "trace", skip(self), ret)]
385+
fn local(&mut self, local: Local) -> VnIndex {
386+
if let Some(value) = self.locals[local] {
387+
return value;
388+
}
389+
let value = self.new_opaque(self.local_decls[local].ty);
390+
self.locals[local] = Some(value);
391+
self.rev_locals_non_ssa.entry(value).or_default().push(local);
392+
value
393+
}
394+
395+
#[instrument(level = "trace", skip(self))]
396+
fn discard_place(&mut self, place: Place<'tcx>) {
397+
let discard_local = |this: &mut Self, local| {
398+
if this.ssa.is_ssa(local) {
399+
return;
400+
}
401+
if let Some(value) = this.locals[local].take() {
402+
this.rev_locals_non_ssa.entry(value).or_default().retain(|l| *l != local);
403+
}
404+
};
405+
if place.is_indirect_first_projection() {
406+
// Non-local mutation maybe invalidate deref.
407+
self.invalidate_derefs();
408+
// Remove stored value from borrowed locals.
409+
for local in self.ssa.borrowed_locals().iter() {
410+
discard_local(self, local);
411+
}
412+
} else {
413+
discard_local(self, place.local);
414+
}
373415
}
374416

375417
fn insert_constant(&mut self, value: Const<'tcx>) -> VnIndex {
@@ -634,7 +676,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
634676
let (mut place_ty, mut value) = match base {
635677
// The base is a local, so we take the local's value and project from it.
636678
AddressBase::Local(local) => {
637-
let local = self.locals[local]?;
679+
let local = self.local(local);
638680
let place_ty = PlaceTy::from_ty(self.ty(local));
639681
(place_ty, local)
640682
}
@@ -740,7 +782,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
740782
// If the projection is indirect, we treat the local as a value, so can replace it with
741783
// another local.
742784
if place.is_indirect_first_projection()
743-
&& let Some(base) = self.locals[place.local]
785+
&& let base = self.local(place.local)
744786
&& let Some(new_local) = self.try_as_local(base, location)
745787
&& place.local != new_local
746788
{
@@ -752,9 +794,8 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
752794

753795
for i in 0..projection.len() {
754796
let elem = projection[i];
755-
if let ProjectionElem::Index(idx_local) = elem
756-
&& let Some(idx) = self.locals[idx_local]
757-
{
797+
if let ProjectionElem::Index(idx_local) = elem {
798+
let idx = self.local(idx_local);
758799
if let Some(offset) = self.evaluated[idx].as_ref()
759800
&& let Some(offset) = self.ecx.read_target_usize(offset).discard_err()
760801
&& let Some(min_length) = offset.checked_add(1)
@@ -790,7 +831,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
790831
let mut place_ref = place.as_ref();
791832

792833
// Invariant: `value` holds the value up-to the `index`th projection excluded.
793-
let Some(mut value) = self.locals[place.local] else { return Err(place_ref) };
834+
let mut value = self.local(place.local);
794835
// Invariant: `value` has type `place_ty`, with optional downcast variant if needed.
795836
let mut place_ty = PlaceTy::from_ty(self.local_decls[place.local].ty);
796837
for (index, proj) in place.projection.iter().enumerate() {
@@ -801,7 +842,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
801842
place_ref = PlaceRef { local, projection: &place.projection[index..] };
802843
}
803844

804-
let Some(proj) = proj.try_map(|value| self.locals[value], |ty| ty) else {
845+
let Some(proj) = proj.try_map(|value| Some(self.local(value)), |ty| ty) else {
805846
return Err(place_ref);
806847
};
807848
let Some(ty_and_value) = self.project(place_ty, value, proj) else {
@@ -1710,11 +1751,17 @@ impl<'tcx> VnState<'_, '_, 'tcx> {
17101751
/// If there is a local which is assigned `index`, and its assignment strictly dominates `loc`,
17111752
/// return it. If you used this local, add it to `reused_locals` to remove storage statements.
17121753
fn try_as_local(&mut self, index: VnIndex, loc: Location) -> Option<Local> {
1713-
let other = self.rev_locals.get(index)?;
1714-
other
1715-
.iter()
1716-
.find(|&&other| self.ssa.assignment_dominates(&self.dominators, other, loc))
1717-
.copied()
1754+
if let Some(ssa) = self.rev_locals_ssa.get(index)
1755+
&& let Some(other) = ssa
1756+
.iter()
1757+
.find(|&&other| self.ssa.assignment_dominates(&self.dominators, other, loc))
1758+
{
1759+
Some(*other)
1760+
} else if let Some(non_ssa) = self.rev_locals_non_ssa.get(&index) {
1761+
non_ssa.first().copied()
1762+
} else {
1763+
None
1764+
}
17181765
}
17191766
}
17201767

@@ -1723,11 +1770,20 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
17231770
self.tcx
17241771
}
17251772

1773+
fn visit_basic_block_data(&mut self, block: BasicBlock, bbdata: &mut BasicBlockData<'tcx>) {
1774+
self.rev_locals_non_ssa.clear();
1775+
for local in self.locals.indices() {
1776+
if !self.ssa.is_ssa(local) {
1777+
self.locals[local] = None;
1778+
}
1779+
}
1780+
self.super_basic_block_data(block, bbdata);
1781+
}
1782+
17261783
fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) {
17271784
self.simplify_place_projection(place, location);
1728-
if context.is_mutating_use() && place.is_indirect() {
1729-
// Non-local mutation maybe invalidate deref.
1730-
self.invalidate_derefs();
1785+
if context.is_mutating_use() {
1786+
self.discard_place(*place);
17311787
}
17321788
self.super_place(place, context, location);
17331789
}
@@ -1766,13 +1822,9 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
17661822
}
17671823
}
17681824

1769-
if lhs.is_indirect() {
1770-
// Non-local mutation maybe invalidate deref.
1771-
self.invalidate_derefs();
1772-
}
1825+
self.discard_place(*lhs);
17731826

17741827
if let Some(local) = lhs.as_local()
1775-
&& self.ssa.is_ssa(local)
17761828
&& let rvalue_ty = rvalue.ty(self.local_decls, self.tcx)
17771829
// FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark
17781830
// `local` as reusable if we have an exact type match.
@@ -1784,14 +1836,13 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
17841836
}
17851837

17861838
fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) {
1787-
if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator {
1788-
if let Some(local) = destination.as_local()
1789-
&& self.ssa.is_ssa(local)
1790-
{
1791-
let ty = self.local_decls[local].ty;
1792-
let opaque = self.new_opaque(ty);
1793-
self.assign(local, opaque);
1794-
}
1839+
self.super_terminator(terminator, location);
1840+
if let Terminator { kind: TerminatorKind::Call { destination, .. }, .. } = terminator
1841+
&& let Some(local) = destination.as_local()
1842+
{
1843+
let ty = self.local_decls[local].ty;
1844+
let opaque = self.new_opaque(ty);
1845+
self.assign(local, opaque);
17951846
}
17961847
// Function calls and ASM may invalidate (nested) derefs. We must handle them carefully.
17971848
// Currently, only preserving derefs for trivial terminators like SwitchInt and Goto.
@@ -1802,7 +1853,6 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, '_, 'tcx> {
18021853
if !safe_to_preserve_derefs {
18031854
self.invalidate_derefs();
18041855
}
1805-
self.super_terminator(terminator, location);
18061856
}
18071857
}
18081858

‎tests/coverage/closure.cov-map‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -74,24 +74,22 @@ Number of file 0 mappings: 66
7474
- Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
7575
Highest counter ID seen: c1
7676

77-
Function name: closure::main::{closure#0}
78-
Raw bytes (51): 0x[01, 01, 01, 01, 05, 09, 01, 28, 05, 00, 06, 01, 01, 0d, 00, 1a, 01, 00, 1d, 00, 1e, 01, 01, 0c, 00, 14, 05, 00, 15, 02, 0a, 02, 02, 09, 00, 0a, 01, 01, 09, 00, 17, 01, 00, 18, 00, 20, 01, 01, 05, 00, 06]
77+
Function name: closure::main::{closure#0} (unused)
78+
Raw bytes (49): 0x[01, 01, 00, 09, 00, 28, 05, 00, 06, 00, 01, 0d, 00, 1a, 00, 00, 1d, 00, 1e, 00, 01, 0c, 00, 14, 00, 00, 15, 02, 0a, 00, 02, 09, 00, 0a, 00, 01, 09, 00, 17, 00, 00, 18, 00, 20, 00, 01, 05, 00, 06]
7979
Number of files: 1
8080
- file 0 => $DIR/closure.rs
81-
Number of expressions: 1
82-
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
81+
Number of expressions: 0
8382
Number of file 0 mappings: 9
84-
- Code(Counter(0)) at (prev + 40, 5) to (start + 0, 6)
85-
- Code(Counter(0)) at (prev + 1, 13) to (start + 0, 26)
86-
- Code(Counter(0)) at (prev + 0, 29) to (start + 0, 30)
87-
- Code(Counter(0)) at (prev + 1, 12) to (start + 0, 20)
88-
- Code(Counter(1)) at (prev + 0, 21) to (start + 2, 10)
89-
- Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 10)
90-
= (c0 - c1)
91-
- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 23)
92-
- Code(Counter(0)) at (prev + 0, 24) to (start + 0, 32)
93-
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 6)
94-
Highest counter ID seen: c1
83+
- Code(Zero) at (prev + 40, 5) to (start + 0, 6)
84+
- Code(Zero) at (prev + 1, 13) to (start + 0, 26)
85+
- Code(Zero) at (prev + 0, 29) to (start + 0, 30)
86+
- Code(Zero) at (prev + 1, 12) to (start + 0, 20)
87+
- Code(Zero) at (prev + 0, 21) to (start + 2, 10)
88+
- Code(Zero) at (prev + 2, 9) to (start + 0, 10)
89+
- Code(Zero) at (prev + 1, 9) to (start + 0, 23)
90+
- Code(Zero) at (prev + 0, 24) to (start + 0, 32)
91+
- Code(Zero) at (prev + 1, 5) to (start + 0, 6)
92+
Highest counter ID seen: (none)
9593

9694
Function name: closure::main::{closure#10} (unused)
9795
Raw bytes (25): 0x[01, 01, 00, 04, 00, 9b, 01, 07, 00, 08, 00, 00, 09, 00, 11, 00, 00, 12, 00, 1e, 00, 00, 20, 00, 21]
@@ -199,24 +197,22 @@ Number of file 0 mappings: 7
199197
- Code(Counter(0)) at (prev + 2, 9) to (start + 0, 10)
200198
Highest counter ID seen: c1
201199

202-
Function name: closure::main::{closure#18}
203-
Raw bytes (51): 0x[01, 01, 01, 01, 05, 09, 01, 19, 0d, 00, 0e, 01, 01, 15, 00, 22, 01, 00, 25, 00, 26, 01, 01, 14, 00, 1c, 05, 00, 1d, 02, 12, 02, 02, 11, 00, 12, 01, 01, 11, 00, 1f, 01, 00, 20, 00, 28, 01, 01, 0d, 00, 0e]
200+
Function name: closure::main::{closure#18} (unused)
201+
Raw bytes (49): 0x[01, 01, 00, 09, 00, 19, 0d, 00, 0e, 00, 01, 15, 00, 22, 00, 00, 25, 00, 26, 00, 01, 14, 00, 1c, 00, 00, 1d, 02, 12, 00, 02, 11, 00, 12, 00, 01, 11, 00, 1f, 00, 00, 20, 00, 28, 00, 01, 0d, 00, 0e]
204202
Number of files: 1
205203
- file 0 => $DIR/closure.rs
206-
Number of expressions: 1
207-
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
204+
Number of expressions: 0
208205
Number of file 0 mappings: 9
209-
- Code(Counter(0)) at (prev + 25, 13) to (start + 0, 14)
210-
- Code(Counter(0)) at (prev + 1, 21) to (start + 0, 34)
211-
- Code(Counter(0)) at (prev + 0, 37) to (start + 0, 38)
212-
- Code(Counter(0)) at (prev + 1, 20) to (start + 0, 28)
213-
- Code(Counter(1)) at (prev + 0, 29) to (start + 2, 18)
214-
- Code(Expression(0, Sub)) at (prev + 2, 17) to (start + 0, 18)
215-
= (c0 - c1)
216-
- Code(Counter(0)) at (prev + 1, 17) to (start + 0, 31)
217-
- Code(Counter(0)) at (prev + 0, 32) to (start + 0, 40)
218-
- Code(Counter(0)) at (prev + 1, 13) to (start + 0, 14)
219-
Highest counter ID seen: c1
206+
- Code(Zero) at (prev + 25, 13) to (start + 0, 14)
207+
- Code(Zero) at (prev + 1, 21) to (start + 0, 34)
208+
- Code(Zero) at (prev + 0, 37) to (start + 0, 38)
209+
- Code(Zero) at (prev + 1, 20) to (start + 0, 28)
210+
- Code(Zero) at (prev + 0, 29) to (start + 2, 18)
211+
- Code(Zero) at (prev + 2, 17) to (start + 0, 18)
212+
- Code(Zero) at (prev + 1, 17) to (start + 0, 31)
213+
- Code(Zero) at (prev + 0, 32) to (start + 0, 40)
214+
- Code(Zero) at (prev + 1, 13) to (start + 0, 14)
215+
Highest counter ID seen: (none)
220216

221217
Function name: closure::main::{closure#19}
222218
Raw bytes (51): 0x[01, 01, 01, 01, 05, 09, 01, 43, 0d, 00, 0e, 01, 01, 15, 00, 22, 01, 00, 25, 00, 26, 01, 01, 14, 00, 1c, 05, 00, 1d, 02, 12, 02, 02, 11, 00, 12, 01, 01, 11, 00, 1f, 01, 00, 20, 00, 28, 01, 01, 0d, 00, 0e]

‎tests/coverage/issue-83601.cov-map‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,22 @@
11
Function name: issue_83601::main
2-
Raw bytes (76): 0x[01, 01, 01, 05, 09, 0e, 01, 06, 01, 00, 0a, 01, 01, 09, 00, 0c, 01, 00, 0f, 00, 15, 01, 01, 05, 00, 0f, 05, 01, 09, 00, 0c, 05, 00, 0f, 00, 15, 05, 01, 05, 00, 0f, 02, 01, 05, 00, 0d, 02, 00, 0e, 00, 14, 02, 01, 05, 00, 0d, 02, 00, 0e, 00, 14, 02, 01, 05, 00, 0d, 02, 00, 0e, 00, 14, 02, 01, 01, 00, 02]
2+
Raw bytes (74): 0x[01, 01, 00, 0e, 01, 06, 01, 00, 0a, 01, 01, 09, 00, 0c, 01, 00, 0f, 00, 15, 01, 01, 05, 00, 0f, 01, 01, 09, 00, 0c, 01, 00, 0f, 00, 15, 01, 01, 05, 00, 0f, 01, 01, 05, 00, 0d, 01, 00, 0e, 00, 14, 01, 01, 05, 00, 0d, 01, 00, 0e, 00, 14, 01, 01, 05, 00, 0d, 01, 00, 0e, 00, 14, 01, 01, 01, 00, 02]
33
Number of files: 1
44
- file 0 => $DIR/issue-83601.rs
5-
Number of expressions: 1
6-
- expression 0 operands: lhs = Counter(1), rhs = Counter(2)
5+
Number of expressions: 0
76
Number of file 0 mappings: 14
87
- Code(Counter(0)) at (prev + 6, 1) to (start + 0, 10)
98
- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 12)
109
- Code(Counter(0)) at (prev + 0, 15) to (start + 0, 21)
1110
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 15)
12-
- Code(Counter(1)) at (prev + 1, 9) to (start + 0, 12)
13-
- Code(Counter(1)) at (prev + 0, 15) to (start + 0, 21)
14-
- Code(Counter(1)) at (prev + 1, 5) to (start + 0, 15)
15-
- Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 13)
16-
= (c1 - c2)
17-
- Code(Expression(0, Sub)) at (prev + 0, 14) to (start + 0, 20)
18-
= (c1 - c2)
19-
- Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 13)
20-
= (c1 - c2)
21-
- Code(Expression(0, Sub)) at (prev + 0, 14) to (start + 0, 20)
22-
= (c1 - c2)
23-
- Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 13)
24-
= (c1 - c2)
25-
- Code(Expression(0, Sub)) at (prev + 0, 14) to (start + 0, 20)
26-
= (c1 - c2)
27-
- Code(Expression(0, Sub)) at (prev + 1, 1) to (start + 0, 2)
28-
= (c1 - c2)
29-
Highest counter ID seen: c1
11+
- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 12)
12+
- Code(Counter(0)) at (prev + 0, 15) to (start + 0, 21)
13+
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 15)
14+
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 13)
15+
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 20)
16+
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 13)
17+
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 20)
18+
- Code(Counter(0)) at (prev + 1, 5) to (start + 0, 13)
19+
- Code(Counter(0)) at (prev + 0, 14) to (start + 0, 20)
20+
- Code(Counter(0)) at (prev + 1, 1) to (start + 0, 2)
21+
Highest counter ID seen: c0
3022

0 commit comments

Comments
 (0)