From 61ede075bfbb1bcbe09595565381ab9c5ef5deec Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 14:21:13 +0000 Subject: [PATCH 1/4] Stop ICEing on impossible predicates. --- compiler/rustc_mir_dataflow/src/value_analysis.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/value_analysis.rs b/compiler/rustc_mir_dataflow/src/value_analysis.rs index 5e2d88f94ca28..bfbfff7e25944 100644 --- a/compiler/rustc_mir_dataflow/src/value_analysis.rs +++ b/compiler/rustc_mir_dataflow/src/value_analysis.rs @@ -846,9 +846,10 @@ impl Map { if let ty::Ref(_, ref_ty, _) | ty::RawPtr(ref_ty, _) = ty.kind() && let ty::Slice(..) = ref_ty.kind() + // The user may have written a predicate like `[T]: Sized` in their where clauses, + // which makes slices scalars. + && self.places[place].value_index.is_none() { - 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; From a175817ea6b554a3baa2debc4bcfdea88e467eeb Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 22:25:03 +0000 Subject: [PATCH 2/4] Avoid cloning state when possible. --- compiler/rustc_mir_transform/src/jump_threading.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_transform/src/jump_threading.rs b/compiler/rustc_mir_transform/src/jump_threading.rs index 23cc0c46e7397..0dad003ee5f42 100644 --- a/compiler/rustc_mir_transform/src/jump_threading.rs +++ b/compiler/rustc_mir_transform/src/jump_threading.rs @@ -270,12 +270,13 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { self.process_switch_int(discr, targets, bb, &mut state); self.find_opportunity(pred, state, cost, depth + 1); } - _ => self.recurse_through_terminator(pred, &state, &cost, depth), + _ => self.recurse_through_terminator(pred, || state, &cost, depth), } - } else { + } else if let &[ref predecessors @ .., last_pred] = &predecessors[..] { for &pred in predecessors { - self.recurse_through_terminator(pred, &state, &cost, depth); + self.recurse_through_terminator(pred, || state.clone(), &cost, depth); } + self.recurse_through_terminator(last_pred, || state, &cost, depth); } let new_tos = &mut self.opportunities[last_non_rec..]; @@ -566,11 +567,12 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { None } - #[instrument(level = "trace", skip(self, cost))] + #[instrument(level = "trace", skip(self, state, cost))] fn recurse_through_terminator( &mut self, bb: BasicBlock, - state: &State>, + // Pass a closure that may clone the state, as we don't want to do it each time. + state: impl FnOnce() -> State>, cost: &CostChecker<'_, 'tcx>, depth: usize, ) { @@ -600,7 +602,7 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { }; // We can recurse through this terminator. - let mut state = state.clone(); + let mut state = state(); if let Some(place_to_flood) = place_to_flood { state.flood_with(place_to_flood.as_ref(), self.map, ConditionSet::default()); } From dec4e985221cfa0a45828903f998ce4631c511d5 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Jun 2024 16:18:03 +0000 Subject: [PATCH 3/4] Move entry point to a method. --- .../rustc_mir_transform/src/jump_threading.rs | 79 ++++++++++--------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_mir_transform/src/jump_threading.rs b/compiler/rustc_mir_transform/src/jump_threading.rs index 0dad003ee5f42..27e506a920bce 100644 --- a/compiler/rustc_mir_transform/src/jump_threading.rs +++ b/compiler/rustc_mir_transform/src/jump_threading.rs @@ -91,43 +91,8 @@ impl<'tcx> MirPass<'tcx> for JumpThreading { opportunities: Vec::new(), }; - for (bb, bbdata) in body.basic_blocks.iter_enumerated() { - debug!(?bb, term = ?bbdata.terminator()); - if bbdata.is_cleanup || loop_headers.contains(bb) { - continue; - } - let Some((discr, targets)) = bbdata.terminator().kind.as_switch() else { continue }; - let Some(discr) = discr.place() else { continue }; - debug!(?discr, ?bb); - - let discr_ty = discr.ty(body, tcx).ty; - let Ok(discr_layout) = finder.ecx.layout_of(discr_ty) else { continue }; - - let Some(discr) = finder.map.find(discr.as_ref()) else { continue }; - debug!(?discr); - - let cost = CostChecker::new(tcx, param_env, None, body); - - let mut state = State::new(ConditionSet::default(), finder.map); - - let conds = if let Some((value, then, else_)) = targets.as_static_if() { - let Some(value) = ScalarInt::try_from_uint(value, discr_layout.size) else { - continue; - }; - arena.alloc_from_iter([ - Condition { value, polarity: Polarity::Eq, target: then }, - Condition { value, polarity: Polarity::Ne, target: else_ }, - ]) - } else { - arena.alloc_from_iter(targets.iter().filter_map(|(value, target)| { - let value = ScalarInt::try_from_uint(value, discr_layout.size)?; - Some(Condition { value, polarity: Polarity::Eq, target }) - })) - }; - let conds = ConditionSet(conds); - state.insert_value_idx(discr, conds, finder.map); - - finder.find_opportunity(bb, state, cost, 0); + for bb in body.basic_blocks.indices() { + finder.start_from_switch(bb); } let opportunities = finder.opportunities; @@ -216,6 +181,46 @@ impl<'tcx, 'a> TOFinder<'tcx, 'a> { } /// Recursion entry point to find threading opportunities. + #[instrument(level = "trace", skip(self))] + fn start_from_switch(&mut self, bb: BasicBlock) -> Option { + let bbdata = &self.body[bb]; + if bbdata.is_cleanup || self.loop_headers.contains(bb) { + return None; + } + let (discr, targets) = bbdata.terminator().kind.as_switch()?; + let discr = discr.place()?; + debug!(?discr, ?bb); + + let discr_ty = discr.ty(self.body, self.tcx).ty; + let discr_layout = self.ecx.layout_of(discr_ty).ok()?; + + let discr = self.map.find(discr.as_ref())?; + debug!(?discr); + + let cost = CostChecker::new(self.tcx, self.param_env, None, self.body); + let mut state = State::new(ConditionSet::default(), self.map); + + let conds = if let Some((value, then, else_)) = targets.as_static_if() { + let value = ScalarInt::try_from_uint(value, discr_layout.size)?; + self.arena.alloc_from_iter([ + Condition { value, polarity: Polarity::Eq, target: then }, + Condition { value, polarity: Polarity::Ne, target: else_ }, + ]) + } else { + self.arena.alloc_from_iter(targets.iter().filter_map(|(value, target)| { + let value = ScalarInt::try_from_uint(value, discr_layout.size)?; + Some(Condition { value, polarity: Polarity::Eq, target }) + })) + }; + let conds = ConditionSet(conds); + state.insert_value_idx(discr, conds, self.map); + + self.find_opportunity(bb, state, cost, 0); + None + } + + /// Recursively walk statements backwards from this bb's terminator to find threading + /// opportunities. #[instrument(level = "trace", skip(self, cost), ret)] fn find_opportunity( &mut self, From c81481fdb91cca96635017e7275139b0f2cd8fe6 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 27 Jun 2024 09:39:37 +0000 Subject: [PATCH 4/4] Move crash test. --- .../116721.rs => ui/mir/sized-slice-predicate-116721.rs} | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename tests/{crashes/116721.rs => ui/mir/sized-slice-predicate-116721.rs} (82%) diff --git a/tests/crashes/116721.rs b/tests/ui/mir/sized-slice-predicate-116721.rs similarity index 82% rename from tests/crashes/116721.rs rename to tests/ui/mir/sized-slice-predicate-116721.rs index fc1a6530bc82b..c6a0aca2da80d 100644 --- a/tests/crashes/116721.rs +++ b/tests/ui/mir/sized-slice-predicate-116721.rs @@ -1,5 +1,6 @@ -//@ known-bug: #116721 +//@ build-pass //@ compile-flags: -Zmir-opt-level=3 --emit=mir + fn hey(it: &[T]) where [T]: Clone,