Skip to content

Improvements to dataflow const-prop #115612

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 9 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1751,6 +1751,13 @@ impl<'tcx> PlaceRef<'tcx> {
}
}

impl From<Local> for PlaceRef<'_> {
#[inline]
fn from(local: Local) -> Self {
PlaceRef { local, projection: &[] }
}
}

impl Debug for Place<'_> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
for elem in self.projection.iter().rev() {
Expand Down
36 changes: 36 additions & 0 deletions compiler/rustc_mir_dataflow/src/value_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,14 @@ impl<V: Clone + HasTop + HasBottom> State<V> {
}
}

/// Retrieve the value stored for a place, or ⊤ if it is not tracked.
pub fn get_len(&self, place: PlaceRef<'_>, map: &Map) -> V {
match map.find_len(place) {
Some(place) => self.get_idx(place, map),
None => V::TOP,
}
}

/// Retrieve the value stored for a place index, or ⊤ if it is not tracked.
pub fn get_idx(&self, place: PlaceIndex, map: &Map) -> V {
match &self.0 {
Expand Down Expand Up @@ -750,6 +758,24 @@ impl Map {
self.value_count += 1;
}

if let Some(ref_ty) = ty.builtin_deref(true) && let ty::Slice(..) = ref_ty.ty.kind() {
assert!(self.places[place].value_index.is_none(), "slices are not scalars");

// Prepend new child to the linked list.
let len = self.places.push(PlaceInfo::new(Some(TrackElem::DerefLen)));
self.places[len].next_sibling = self.places[place].first_child;
self.places[place].first_child = Some(len);

let old = self.projections.insert((place, TrackElem::DerefLen), len);
assert!(old.is_none());

// Allocate a value slot if it doesn't have one.
if self.places[len].value_index.is_none() {
self.places[len].value_index = Some(self.value_count.into());
self.value_count += 1;
}
}

// Recurse with all fields of this place.
iter_fields(ty, tcx, param_env, |variant, field, ty| {
worklist.push_back((
Expand Down Expand Up @@ -818,6 +844,11 @@ impl Map {
self.find_extra(place, [TrackElem::Discriminant])
}

/// Locates the given place and applies `DerefLen`, if it exists in the tree.
pub fn find_len(&self, place: PlaceRef<'_>) -> Option<PlaceIndex> {
self.find_extra(place, [TrackElem::DerefLen])
}

/// Iterate over all direct children.
pub fn children(&self, parent: PlaceIndex) -> impl Iterator<Item = PlaceIndex> + '_ {
Children::new(self, parent)
Expand Down Expand Up @@ -969,6 +1000,8 @@ pub enum TrackElem {
Field(FieldIdx),
Variant(VariantIdx),
Discriminant,
// Length of a slice.
DerefLen,
}

impl<V, T> TryFrom<ProjectionElem<V, T>> for TrackElem {
Expand Down Expand Up @@ -1108,6 +1141,9 @@ fn debug_with_context_rec<V: Debug + Eq>(
format!("{}.{}", place_str, field.index())
}
}
TrackElem::DerefLen => {
format!("Len(*{})", place_str)
}
};
debug_with_context_rec(child, &child_place_str, new, old, map, f)?;
}
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_mir_transform/src/dataflow_const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,23 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
}
}
}
Rvalue::Cast(
CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize),
operand,
_,
) => {
let pointer = self.handle_operand(operand, state);
state.assign(target.as_ref(), pointer, self.map());

if let Some(target_len) = self.map().find_len(target.as_ref())
&& let operand_ty = operand.ty(self.local_decls, self.tcx)
&& let Some(operand_ty) = operand_ty.builtin_deref(true)
&& let ty::Array(_, len) = operand_ty.ty.kind()
&& let Some(len) = ConstantKind::Ty(*len).eval(self.tcx, self.param_env).try_to_scalar_int()
{
state.insert_value_idx(target_len, FlatSet::Elem(len), self.map());
}
}
_ => self.super_assign(target, rvalue, state),
}
}
Expand All @@ -194,6 +211,19 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
state: &mut State<Self::Value>,
) -> ValueOrPlace<Self::Value> {
let val = match rvalue {
Rvalue::Len(place) => {
let place_ty = place.ty(self.local_decls, self.tcx);
if let ty::Array(_, len) = place_ty.ty.kind() {
ConstantKind::Ty(*len)
.eval(self.tcx, self.param_env)
.try_to_scalar_int()
.map_or(FlatSet::Top, FlatSet::Elem)
} else if let [ProjectionElem::Deref] = place.projection[..] {
state.get_len(place.local.into(), self.map())
} else {
FlatSet::Top
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another option. If the slice is from a constant, we could even read the length from the constant, without it needing to be derived from an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we'd still have Len(*_local) where _local holds the slice. For that case, I'd rather directly implement general assignments x = const where the RHS is not a scalar.

}
Rvalue::Cast(CastKind::IntToInt | CastKind::IntToFloat, operand, ty) => {
match self.eval_operand(operand, state) {
FlatSet::Elem(op) => self
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
StorageLive(_5);
StorageLive(_6);
_6 = const 3_usize;
_7 = const 3_usize;
_7 = Len((*_1));
- _8 = Lt(_6, _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind unreachable];
+ _8 = const false;
+ assert(const false, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 3_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 3_usize, _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 3_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
StorageLive(_5);
StorageLive(_6);
_6 = const 3_usize;
_7 = const 3_usize;
_7 = Len((*_1));
- _8 = Lt(_6, _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind continue];
+ _8 = const false;
+ assert(const false, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 3_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 3_usize, _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 3_usize) -> [success: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
StorageLive(_5);
StorageLive(_6);
_6 = const 3_usize;
_7 = const 3_usize;
_7 = Len((*_1));
- _8 = Lt(_6, _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind unreachable];
+ _8 = const false;
+ assert(const false, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 3_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 3_usize, _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 3_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@
StorageLive(_5);
StorageLive(_6);
_6 = const 3_usize;
_7 = const 3_usize;
_7 = Len((*_1));
- _8 = Lt(_6, _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind continue];
+ _8 = const false;
+ assert(const false, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 3_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 3_usize, _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 3_usize) -> [success: bb1, unwind continue];
}

bb1: {
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/bad_op_unsafe_oob_for_slices.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// unit-test: ConstProp
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// compile-flags: -Zmir-enable-passes=+NormalizeArrayLen

// EMIT_MIR_FOR_EACH_BIT_WIDTH

// EMIT_MIR bad_op_unsafe_oob_for_slices.main.ConstProp.diff
#[allow(unconditional_panic)]
fn main() {
Expand Down
1 change: 0 additions & 1 deletion tests/mir-opt/const_prop/large_array_index.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// unit-test: ConstProp
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// compile-flags: -Zmir-enable-passes=+NormalizeArrayLen
// EMIT_MIR_FOR_EACH_BIT_WIDTH

// EMIT_MIR large_array_index.main.ConstProp.diff
Expand Down
1 change: 0 additions & 1 deletion tests/mir-opt/const_prop/repeat.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// unit-test: ConstProp
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// compile-flags: -Zmir-enable-passes=+NormalizeArrayLen
// EMIT_MIR_FOR_EACH_BIT_WIDTH

// EMIT_MIR repeat.main.ConstProp.diff
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
- // MIR for `main` before DataflowConstProp
+ // MIR for `main` after DataflowConstProp

fn main() -> () {
let mut _0: ();
let _1: u32;
let mut _2: [u32; 4];
let _3: usize;
let mut _4: usize;
let mut _5: bool;
scope 1 {
debug x => _1;
}

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
- _5 = Lt(_3, _4);
- assert(move _5, "index out of bounds: the length is {} but the index is {}", move _4, _3) -> [success: bb1, unwind unreachable];
+ _4 = const 4_usize;
+ _5 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 4_usize, const 2_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
_1 = _2[_3];
StorageDead(_3);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
return;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
- // MIR for `main` before DataflowConstProp
+ // MIR for `main` after DataflowConstProp

fn main() -> () {
let mut _0: ();
let _1: u32;
let mut _2: [u32; 4];
let _3: usize;
let mut _4: usize;
let mut _5: bool;
scope 1 {
debug x => _1;
}

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
- _5 = Lt(_3, _4);
- assert(move _5, "index out of bounds: the length is {} but the index is {}", move _4, _3) -> [success: bb1, unwind continue];
+ _4 = const 4_usize;
+ _5 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 4_usize, const 2_usize) -> [success: bb1, unwind continue];
}

bb1: {
_1 = _2[_3];
StorageDead(_3);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
return;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
- // MIR for `main` before DataflowConstProp
+ // MIR for `main` after DataflowConstProp

fn main() -> () {
let mut _0: ();
let _1: u32;
let mut _2: [u32; 4];
let _3: usize;
let mut _4: usize;
let mut _5: bool;
scope 1 {
debug x => _1;
}

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
- _5 = Lt(_3, _4);
- assert(move _5, "index out of bounds: the length is {} but the index is {}", move _4, _3) -> [success: bb1, unwind unreachable];
+ _4 = const 4_usize;
+ _5 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 4_usize, const 2_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
_1 = _2[_3];
StorageDead(_3);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
return;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
- // MIR for `main` before DataflowConstProp
+ // MIR for `main` after DataflowConstProp

fn main() -> () {
let mut _0: ();
let _1: u32;
let mut _2: [u32; 4];
let _3: usize;
let mut _4: usize;
let mut _5: bool;
scope 1 {
debug x => _1;
}

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = [const 0_u32, const 1_u32, const 2_u32, const 3_u32];
StorageLive(_3);
_3 = const 2_usize;
- _4 = Len(_2);
- _5 = Lt(_3, _4);
- assert(move _5, "index out of bounds: the length is {} but the index is {}", move _4, _3) -> [success: bb1, unwind continue];
+ _4 = const 4_usize;
+ _5 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 4_usize, const 2_usize) -> [success: bb1, unwind continue];
}

bb1: {
_1 = _2[_3];
StorageDead(_3);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
return;
}
}

8 changes: 8 additions & 0 deletions tests/mir-opt/dataflow-const-prop/array_index.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// unit-test: DataflowConstProp
// EMIT_MIR_FOR_EACH_BIT_WIDTH

// EMIT_MIR array_index.main.DataflowConstProp.diff
fn main() {
let x: u32 = [0, 1, 2, 3][2];
}
Loading