Skip to content

Commit

Permalink
Auto merge of #101168 - jachris:dataflow-const-prop, r=oli-obk
Browse files Browse the repository at this point in the history
Add new MIR constant propagation based on dataflow analysis

The current constant propagation in `rustc_mir_transform/src/const_prop.rs` fails to handle many cases that would be expected from a constant propagation optimization. For example:
```rust
let x = if true { 0 } else { 0 };
```
This pull request adds a new constant propagation MIR optimization pass based on the existing dataflow analysis framework. Since most of the analysis is not unique to constant propagation, a generic framework has been extracted. It works on top of the existing framework and could be reused for other optimzations.

Closes #80038. Closes #81605.

## Todo
### Essential
- [x] [Writes to inactive enum variants](#101168 (review)). Resolved by rejecting the registration of places with downcast projections for now. Could be improved by flooding other variants if mutable access to a variant is observed.
- [X] Handle [`StatementKind::CopyNonOverlapping`](#101168 (comment)). Resolved by flooding the destination.
- [x] Handle `UnsafeCell` / `!Freeze` correctly.
- [X] Overflow propagation of `CheckedBinaryOp`: Decided to not propagate if overflow flag is `true` (`false` will still be propagated)
- [x] More documentation in general.
- [x] Arguments for correctness, documentation of necessary assumptions.
- [x] Better performance, or alternatively, require `-Zmir-opt-level=3` for now.

### Extra
- [x]  Add explicit unreachability, i.e. upgrading the lattice from $\mathbb{P} \to \mathbb{V}$ to $\set{\bot} \cup (\mathbb{P} \to \mathbb{V})$.
- [x] Use storage statements to improve precision.
- [ ] Consider opening issue for duplicate diagnostics: #101168 (comment)
- [ ] Flood moved-from places with $\bot$ (requires some changes for places with tracked projections).
- [ ] Add downcast projections back in.
- [ ] [Algebraic simplifications](#101168 (comment)) (possibly with a shared API; done by old const prop).
- [ ] Propagation through slices / arrays.
- [ ] Find other optimizations that are done by old `const_prop.rs`, but not by this one.
  • Loading branch information
bors committed Nov 15, 2022
2 parents ca92d90 + 24d2e90 commit 357f660
Show file tree
Hide file tree
Showing 43 changed files with 2,441 additions and 25 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_graphviz/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,11 @@ pub trait Labeller<'a> {
/// Escape tags in such a way that it is suitable for inclusion in a
/// Graphviz HTML label.
pub fn escape_html(s: &str) -> String {
s.replace('&', "&amp;").replace('\"', "&quot;").replace('<', "&lt;").replace('>', "&gt;")
s.replace('&', "&amp;")
.replace('\"', "&quot;")
.replace('<', "&lt;")
.replace('>', "&gt;")
.replace('\n', "<br align=\"left\"/>")
}

impl<'a> LabelText<'a> {
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/mir/generic_graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<
write!(
w,
r#"<tr><td align="left" balign="left">{}</td></tr>"#,
dot::escape_html(&section).replace('\n', "<br/>")
dot::escape_html(&section)
)?;
}

Expand All @@ -147,7 +147,7 @@ impl<
let src = self.node(source);
let trg = self.node(target);
let escaped_edge_label = if let Some(edge_label) = edge_labels.get(index) {
dot::escape_html(edge_label).replace('\n', r#"<br align="left"/>"#)
dot::escape_html(edge_label)
} else {
"".to_owned()
};
Expand All @@ -162,8 +162,7 @@ impl<
where
W: Write,
{
let lines = label.split('\n').map(|s| dot::escape_html(s)).collect::<Vec<_>>();
let escaped_label = lines.join(r#"<br align="left"/>"#);
let escaped_label = dot::escape_html(label);
writeln!(w, r#" label=<<br/><br/>{}<br align="left"/><br/><br/><br/>>;"#, escaped_label)
}

Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,15 @@ impl PlaceContext {
)
}

/// Returns `true` if this place context represents an address-of.
pub fn is_address_of(&self) -> bool {
matches!(
self,
PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf)
| PlaceContext::MutatingUse(MutatingUseContext::AddressOf)
)
}

/// Returns `true` if this place context represents a storage live or storage dead marker.
#[inline]
pub fn is_storage_marker(&self) -> bool {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_mir_dataflow/src/framework/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,10 @@ where
r#"<td colspan="{colspan}" {fmt} align="left">{state}</td>"#,
colspan = this.style.num_state_columns(),
fmt = fmt,
state = format!("{:?}", DebugWithAdapter { this: state, ctxt: analysis }),
state = dot::escape_html(&format!(
"{:?}",
DebugWithAdapter { this: state, ctxt: analysis }
)),
)
})
}
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_mir_dataflow/src/framework/lattice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ pub trait MeetSemiLattice: Eq {
fn meet(&mut self, other: &Self) -> bool;
}

/// A set that has a "bottom" element, which is less than or equal to any other element.
pub trait HasBottom {
fn bottom() -> Self;
}

/// A set that has a "top" element, which is greater than or equal to any other element.
pub trait HasTop {
fn top() -> Self;
}

/// A `bool` is a "two-point" lattice with `true` as the top element and `false` as the bottom:
///
/// ```text
Expand Down Expand Up @@ -102,6 +112,18 @@ impl MeetSemiLattice for bool {
}
}

impl HasBottom for bool {
fn bottom() -> Self {
false
}
}

impl HasTop for bool {
fn top() -> Self {
true
}
}

/// A tuple (or list) of lattices is itself a lattice whose least upper bound is the concatenation
/// of the least upper bounds of each element of the tuple (or list).
///
Expand Down Expand Up @@ -250,3 +272,15 @@ impl<T: Clone + Eq> MeetSemiLattice for FlatSet<T> {
true
}
}

impl<T> HasBottom for FlatSet<T> {
fn bottom() -> Self {
Self::Bottom
}
}

impl<T> HasTop for FlatSet<T> {
fn top() -> Self {
Self::Top
}
}
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub mod move_paths;
pub mod rustc_peek;
pub mod storage;
pub mod un_derefer;
pub mod value_analysis;

pub(crate) mod indexes {
pub(crate) use super::move_paths::MovePathIndex;
Expand Down

0 comments on commit 357f660

Please sign in to comment.