Skip to content

Commit f9c2421

Browse files
borsbors
committed
Auto merge of #119439 - cjgillot:gvn-faster, r=oli-obk
Avoid some redundant work in GVN The first 2 commits are about reducing the perf effect. Third commit avoids doing redundant work: is a local is SSA, it already has been simplified, and the resulting value is in `self.locals`. No need to call any code on it. The last commit avoids removing some storage statements. r? wg-mir-opt
2 parents 714b29a + 7e39100 commit f9c2421

File tree

37 files changed

+168
-221
lines changed

37 files changed

+168
-221
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
8585
use rustc_const_eval::interpret::{intern_const_alloc_for_constprop, MemoryKind};
8686
use rustc_const_eval::interpret::{ImmTy, InterpCx, OpTy, Projectable, Scalar};
87-
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
87+
use rustc_data_structures::fx::FxIndexSet;
8888
use rustc_data_structures::graph::dominators::Dominators;
8989
use rustc_hir::def::DefKind;
9090
use rustc_index::bit_set::BitSet;
@@ -99,6 +99,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt, TypeAndMut};
9999
use rustc_span::def_id::DefId;
100100
use rustc_span::DUMMY_SP;
101101
use rustc_target::abi::{self, Abi, Size, VariantIdx, FIRST_VARIANT};
102+
use smallvec::SmallVec;
102103
use std::borrow::Cow;
103104

104105
use crate::dataflow_const_prop::DummyMachine;
@@ -238,14 +239,18 @@ struct VnState<'body, 'tcx> {
238239
local_decls: &'body LocalDecls<'tcx>,
239240
/// Value stored in each local.
240241
locals: IndexVec<Local, Option<VnIndex>>,
241-
/// First local to be assigned that value.
242-
rev_locals: FxHashMap<VnIndex, Vec<Local>>,
242+
/// Locals that are assigned that value.
243+
// This vector does not hold all the values of `VnIndex` that we create.
244+
// It stops at the largest value created in the first phase of collecting assignments.
245+
rev_locals: IndexVec<VnIndex, SmallVec<[Local; 1]>>,
243246
values: FxIndexSet<Value<'tcx>>,
244247
/// Values evaluated as constants if possible.
245248
evaluated: IndexVec<VnIndex, Option<OpTy<'tcx>>>,
246249
/// Counter to generate different values.
247250
/// This is an option to stop creating opaques during replacement.
248251
next_opaque: Option<usize>,
252+
/// Cache the value of the `unsized_locals` features, to avoid fetching it repeatedly in a loop.
253+
feature_unsized_locals: bool,
249254
ssa: &'body SsaLocals,
250255
dominators: &'body Dominators<BasicBlock>,
251256
reused_locals: BitSet<Local>,
@@ -265,10 +270,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
265270
param_env,
266271
local_decls,
267272
locals: IndexVec::from_elem(None, local_decls),
268-
rev_locals: FxHashMap::default(),
273+
rev_locals: IndexVec::default(),
269274
values: FxIndexSet::default(),
270275
evaluated: IndexVec::new(),
271276
next_opaque: Some(0),
277+
feature_unsized_locals: tcx.features().unsized_locals,
272278
ssa,
273279
dominators,
274280
reused_locals: BitSet::new_empty(local_decls.len()),
@@ -316,10 +322,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
316322
self.locals[local] = Some(value);
317323

318324
// Only register the value if its type is `Sized`, as we will emit copies of it.
319-
let is_sized = !self.tcx.features().unsized_locals
325+
let is_sized = !self.feature_unsized_locals
320326
|| self.local_decls[local].ty.is_sized(self.tcx, self.param_env);
321327
if is_sized {
322-
self.rev_locals.entry(value).or_default().push(local);
328+
self.rev_locals.ensure_contains_elem(value, SmallVec::new);
329+
self.rev_locals[value].push(local);
323330
}
324331
}
325332

@@ -630,6 +637,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
630637
if place.is_indirect()
631638
&& let Some(base) = self.locals[place.local]
632639
&& let Some(new_local) = self.try_as_local(base, location)
640+
&& place.local != new_local
633641
{
634642
place.local = new_local;
635643
self.reused_locals.insert(new_local);
@@ -639,18 +647,20 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
639647

640648
for i in 0..projection.len() {
641649
let elem = projection[i];
642-
if let ProjectionElem::Index(idx) = elem
643-
&& let Some(idx) = self.locals[idx]
650+
if let ProjectionElem::Index(idx_local) = elem
651+
&& let Some(idx) = self.locals[idx_local]
644652
{
645653
if let Some(offset) = self.evaluated[idx].as_ref()
646654
&& let Ok(offset) = self.ecx.read_target_usize(offset)
647655
&& let Some(min_length) = offset.checked_add(1)
648656
{
649657
projection.to_mut()[i] =
650658
ProjectionElem::ConstantIndex { offset, min_length, from_end: false };
651-
} else if let Some(new_idx) = self.try_as_local(idx, location) {
652-
projection.to_mut()[i] = ProjectionElem::Index(new_idx);
653-
self.reused_locals.insert(new_idx);
659+
} else if let Some(new_idx_local) = self.try_as_local(idx, location)
660+
&& idx_local != new_idx_local
661+
{
662+
projection.to_mut()[i] = ProjectionElem::Index(new_idx_local);
663+
self.reused_locals.insert(new_idx_local);
654664
}
655665
}
656666
}
@@ -986,11 +996,11 @@ impl<'tcx> VnState<'_, 'tcx> {
986996
/// If there is a local which is assigned `index`, and its assignment strictly dominates `loc`,
987997
/// return it.
988998
fn try_as_local(&mut self, index: VnIndex, loc: Location) -> Option<Local> {
989-
let other = self.rev_locals.get(&index)?;
999+
let other = self.rev_locals.get(index)?;
9901000
other
9911001
.iter()
1002+
.find(|&&other| self.ssa.assignment_dominates(self.dominators, other, loc))
9921003
.copied()
993-
.find(|&other| self.ssa.assignment_dominates(self.dominators, other, loc))
9941004
}
9951005
}
9961006

@@ -1008,23 +1018,32 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
10081018
}
10091019

10101020
fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) {
1011-
if let StatementKind::Assign(box (_, ref mut rvalue)) = stmt.kind
1021+
if let StatementKind::Assign(box (ref mut lhs, ref mut rvalue)) = stmt.kind {
1022+
self.simplify_place_projection(lhs, location);
1023+
10121024
// Do not try to simplify a constant, it's already in canonical shape.
1013-
&& !matches!(rvalue, Rvalue::Use(Operand::Constant(_)))
1014-
{
1015-
if let Some(value) = self.simplify_rvalue(rvalue, location) {
1016-
if let Some(const_) = self.try_as_constant(value) {
1017-
*rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
1018-
} else if let Some(local) = self.try_as_local(value, location)
1019-
&& *rvalue != Rvalue::Use(Operand::Move(local.into()))
1020-
{
1021-
*rvalue = Rvalue::Use(Operand::Copy(local.into()));
1022-
self.reused_locals.insert(local);
1023-
}
1025+
if matches!(rvalue, Rvalue::Use(Operand::Constant(_))) {
1026+
return;
10241027
}
1025-
} else {
1026-
self.super_statement(stmt, location);
1028+
1029+
let value = lhs
1030+
.as_local()
1031+
.and_then(|local| self.locals[local])
1032+
.or_else(|| self.simplify_rvalue(rvalue, location));
1033+
let Some(value) = value else { return };
1034+
1035+
if let Some(const_) = self.try_as_constant(value) {
1036+
*rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
1037+
} else if let Some(local) = self.try_as_local(value, location)
1038+
&& *rvalue != Rvalue::Use(Operand::Move(local.into()))
1039+
{
1040+
*rvalue = Rvalue::Use(Operand::Copy(local.into()));
1041+
self.reused_locals.insert(local);
1042+
}
1043+
1044+
return;
10271045
}
1046+
self.super_statement(stmt, location);
10281047
}
10291048
}
10301049

‎tests/mir-opt/const_allocation.main.GVN.after.32bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC9: &&[(Option<i32>, &[&str])]};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_allocation.main.GVN.after.64bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC9: &&[(Option<i32>, &[&str])]};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_allocation2.main.GVN.after.32bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC9: &&[(Option<i32>, &[&u8])]};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_allocation2.main.GVN.after.64bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC9: &&[(Option<i32>, &[&u8])]};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_allocation3.main.GVN.after.32bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC4: &&Packed};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_allocation3.main.GVN.after.64bit.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ fn main() -> () {
77

88
bb0: {
99
StorageLive(_1);
10-
nop;
10+
StorageLive(_2);
1111
_2 = const {ALLOC2: &&Packed};
1212
_1 = (*_2);
13-
nop;
13+
StorageDead(_2);
1414
StorageDead(_1);
1515
_0 = const ();
1616
return;

‎tests/mir-opt/const_prop/address_of_pair.fn0.GVN.diff‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@
2424
bb0: {
2525
StorageLive(_2);
2626
- _2 = (const 1_i32, const false);
27-
- StorageLive(_3);
2827
+ _2 = const (1_i32, false);
29-
+ nop;
28+
StorageLive(_3);
3029
_3 = &raw mut (_2.1: bool);
3130
- _2 = (const 1_i32, const false);
3231
+ _2 = const (1_i32, false);
@@ -42,9 +41,8 @@
4241
StorageDead(_6);
4342
_0 = _5;
4443
- StorageDead(_5);
45-
- StorageDead(_3);
46-
+ nop;
4744
+ nop;
45+
StorageDead(_3);
4846
StorageDead(_2);
4947
return;
5048
}

‎tests/mir-opt/const_prop/bad_op_unsafe_oob_for_slices.main.GVN.32bit.panic-abort.diff‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,15 @@
2222
}
2323

2424
bb0: {
25-
- StorageLive(_1);
26-
+ nop;
25+
StorageLive(_1);
2726
StorageLive(_2);
28-
- StorageLive(_3);
29-
+ nop;
27+
StorageLive(_3);
3028
_9 = const _;
3129
_3 = &(*_9);
3230
_2 = &raw const (*_3);
3331
_1 = move _2 as *const [i32] (PointerCoercion(Unsize));
3432
StorageDead(_2);
35-
- StorageDead(_3);
36-
+ nop;
33+
StorageDead(_3);
3734
StorageLive(_5);
3835
StorageLive(_6);
3936
_6 = const 3_usize;
@@ -50,8 +47,7 @@
5047
StorageDead(_6);
5148
_0 = const ();
5249
StorageDead(_5);
53-
- StorageDead(_1);
54-
+ nop;
50+
StorageDead(_1);
5551
return;
5652
}
5753
}

‎tests/mir-opt/const_prop/bad_op_unsafe_oob_for_slices.main.GVN.32bit.panic-unwind.diff‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,15 @@
2222
}
2323

2424
bb0: {
25-
- StorageLive(_1);
26-
+ nop;
25+
StorageLive(_1);
2726
StorageLive(_2);
28-
- StorageLive(_3);
29-
+ nop;
27+
StorageLive(_3);
3028
_9 = const _;
3129
_3 = &(*_9);
3230
_2 = &raw const (*_3);
3331
_1 = move _2 as *const [i32] (PointerCoercion(Unsize));
3432
StorageDead(_2);
35-
- StorageDead(_3);
36-
+ nop;
33+
StorageDead(_3);
3734
StorageLive(_5);
3835
StorageLive(_6);
3936
_6 = const 3_usize;
@@ -50,8 +47,7 @@
5047
StorageDead(_6);
5148
_0 = const ();
5249
StorageDead(_5);
53-
- StorageDead(_1);
54-
+ nop;
50+
StorageDead(_1);
5551
return;
5652
}
5753
}

0 commit comments

Comments
 (0)