Improvements to dataflow const-prop #115612
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
| pub fn try_to_scalar_int(self) -> Option<ScalarInt> { | ||
| Some(self.try_to_scalar()?.assert_int()) | ||
| self.try_to_scalar()?.try_to_int().ok() | ||
| } |
There was a problem hiding this comment.
I think we did this on purpose, but since it's undocumented and a footgun 👍
| let old = self.projections.insert((place, TrackElem::Discriminant), discr); | ||
| assert!(old.is_none()); | ||
|
|
||
| // Allocate a value slot if it doesn't have one. |
There was a problem hiding this comment.
this comment is outdated. It's now "since it doesn't have one"
| match self.eval_operand(operand, state) { | ||
| FlatSet::Elem(op) => self.wrap_immediate(*op), | ||
| FlatSet::Bottom => FlatSet::Bottom, | ||
| FlatSet::Top => FlatSet::Top, | ||
| } |
There was a problem hiding this comment.
FlatSet should probably have a map method
There was a problem hiding this comment.
The use case is so rare, I'm not sure it's worth it.
There was a problem hiding this comment.
not important, just saw 3 occurrences of this code snippet ^^
| 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 | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@bors r+ |
bors
added
S-waiting-on-bors
| assert!(old.is_none()); | ||
|
|
||
| // Allocate a value slot since it doesn't have one. | ||
| assert!( self.places[len].value_index.is_none() ); |
There was a problem hiding this comment.
rustfmt skipped trailing spaces, but i can't repro this locally.
There was a problem hiding this comment.
Ah, let chains strikes again:
fn foo() {
if let Some(ref_ty) = x && let ty::Slice(..) = y {
assert!( self.places[len].value_index.is_none() );
}
}
Improvements to dataflow const-prop Partially cherry-picked from rust-lang#110719 r? `@oli-obk` cc `@jachris`
|
☀️ Test successful - checks-actions |
|
👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward |
|
@bors retry |
|
Finished benchmarking commit (b84d659): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 628.962s -> 628.303s (-0.10%) |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (3cd97ed): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 629.777s -> 629.933s (0.02%) |
Partially cherry-picked from #110719
r? @oli-obk
cc @jachris