Skip to content

Commit 8494e68

Browse files
Auto merge of #143852 - ashivaram23:switch_int_data, r=<try>
Collect `SwitchInt` target `VariantIdx`s while building `MaybePlacesSwitchIntData` - Filter discriminants involved in a `SwitchInt` and map their values to `VariantIdx` while building `MaybePlacesSwitchIntData` instead of in `apply_effects_in_block` using `next_discr`. This is easier after #143769. - Use that `Vec` when handling the `otherwise` target instead of making a new `SmallVec`. This could make `MaybeInitializedPlaces`/`MaybeUninitializedPlaces` analysis slightly more efficient, particularly after #142707.
2 parents bfc046a + 04c8592 commit 8494e68

File tree

5 files changed

+71
-55
lines changed

5 files changed

+71
-55
lines changed

compiler/rustc_middle/src/mir/basic_blocks.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::sync::OnceLock;
22

3+
use rustc_abi::VariantIdx;
34
use rustc_data_structures::graph;
45
use rustc_data_structures::graph::dominators::{Dominators, dominators};
56
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
@@ -23,7 +24,7 @@ type Predecessors = IndexVec<BasicBlock, SmallVec<[BasicBlock; 4]>>;
2324
#[derive(Debug, Clone, Copy)]
2425
pub enum SwitchTargetValue {
2526
// A normal switch value.
26-
Normal(u128),
27+
Normal(VariantIdx),
2728
// The final "otherwise" fallback value.
2829
Otherwise,
2930
}

compiler/rustc_mir_dataflow/src/drop_flag_effects.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use rustc_abi::VariantIdx;
22
use rustc_middle::mir::{self, Body, Location, Terminator, TerminatorKind};
3-
use smallvec::SmallVec;
43
use tracing::debug;
54

65
use super::move_paths::{InitKind, LookupResult, MoveData, MovePathIndex};
@@ -158,15 +157,17 @@ where
158157

159158
/// Indicates which variants are inactive at a `SwitchInt` edge by listing their `VariantIdx`s or
160159
/// specifying the single active variant's `VariantIdx`.
161-
pub(crate) enum InactiveVariants {
162-
Inactives(SmallVec<[VariantIdx; 4]>),
160+
pub(crate) enum InactiveVariants<'a> {
161+
Inactives(&'a [(VariantIdx, mir::BasicBlock)]),
163162
Active(VariantIdx),
164163
}
165164

166-
impl InactiveVariants {
165+
impl InactiveVariants<'_> {
167166
fn contains(&self, variant_idx: VariantIdx) -> bool {
168167
match self {
169-
InactiveVariants::Inactives(inactives) => inactives.contains(&variant_idx),
168+
InactiveVariants::Inactives(inactives) => {
169+
inactives.iter().any(|(idx, _)| *idx == variant_idx)
170+
}
170171
InactiveVariants::Active(active) => variant_idx != *active,
171172
}
172173
}
@@ -177,7 +178,7 @@ impl InactiveVariants {
177178
pub(crate) fn on_all_inactive_variants<'tcx>(
178179
move_data: &MoveData<'tcx>,
179180
enum_place: mir::Place<'tcx>,
180-
inactive_variants: &InactiveVariants,
181+
inactive_variants: &InactiveVariants<'_>,
181182
mut handle_inactive_variant: impl FnMut(MovePathIndex),
182183
) {
183184
let LookupResult::Exact(enum_mpi) = move_data.rev_lookup.find(enum_place.as_ref()) else {

compiler/rustc_mir_dataflow/src/framework/direction.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ impl Direction for Backward {
113113
propagate(pred, &tmp);
114114
}
115115

116-
mir::TerminatorKind::SwitchInt { ref discr, .. } => {
117-
if let Some(_data) = analysis.get_switch_int_data(pred, discr) {
116+
mir::TerminatorKind::SwitchInt { ref targets, ref discr } => {
117+
if let Some(_data) = analysis.get_switch_int_data(pred, discr, targets) {
118118
bug!(
119119
"SwitchInt edge effects are unsupported in backward dataflow analyses"
120120
);
@@ -283,23 +283,22 @@ impl Direction for Forward {
283283
}
284284
}
285285
TerminatorEdges::SwitchInt { targets, discr } => {
286-
if let Some(mut data) = analysis.get_switch_int_data(block, discr) {
286+
if let Some(data) = analysis.get_switch_int_data(block, discr, targets) {
287287
let mut tmp = analysis.bottom_value(body);
288-
for (value, target) in targets.iter() {
288+
for (variant_idx, target) in A::switch_int_target_variants(&data) {
289289
tmp.clone_from(exit_state);
290-
let value = SwitchTargetValue::Normal(value);
291-
analysis.apply_switch_int_edge_effect(&mut data, &mut tmp, value, targets);
292-
propagate(target, &tmp);
290+
let value = SwitchTargetValue::Normal(*variant_idx);
291+
analysis.apply_switch_int_edge_effect(&data, &mut tmp, value);
292+
propagate(*target, &tmp);
293293
}
294294

295295
// Once we get to the final, "otherwise" branch, there is no need to preserve
296296
// `exit_state`, so pass it directly to `apply_switch_int_edge_effect` to save
297297
// a clone of the dataflow state.
298298
analysis.apply_switch_int_edge_effect(
299-
&mut data,
299+
&data,
300300
exit_state,
301301
SwitchTargetValue::Otherwise,
302-
targets,
303302
);
304303
propagate(targets.otherwise(), exit_state);
305304
} else {

compiler/rustc_mir_dataflow/src/framework/mod.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
3535
use std::cmp::Ordering;
3636

37+
use rustc_abi::VariantIdx;
3738
use rustc_data_structures::work_queue::WorkQueue;
3839
use rustc_index::bit_set::{DenseBitSet, MixedBitSet};
3940
use rustc_index::{Idx, IndexVec};
@@ -214,17 +215,24 @@ pub trait Analysis<'tcx> {
214215
&mut self,
215216
_block: mir::BasicBlock,
216217
_discr: &mir::Operand<'tcx>,
218+
_targets: &mir::SwitchTargets,
217219
) -> Option<Self::SwitchIntData> {
218220
None
219221
}
220222

223+
/// Returns an iterator over `SwitchInt` target variants stored in `Self::SwitchIntData`.
224+
fn switch_int_target_variants(
225+
_data: &Self::SwitchIntData,
226+
) -> impl Iterator<Item = &(VariantIdx, mir::BasicBlock)> {
227+
[].iter()
228+
}
229+
221230
/// See comments on `get_switch_int_data`.
222231
fn apply_switch_int_edge_effect(
223232
&mut self,
224-
_data: &mut Self::SwitchIntData,
233+
_data: &Self::SwitchIntData,
225234
_state: &mut Self::Domain,
226235
_value: SwitchTargetValue,
227-
_targets: &mir::SwitchTargets,
228236
) {
229237
unreachable!();
230238
}

compiler/rustc_mir_dataflow/src/impls/initialized.rs

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ use rustc_middle::bug;
77
use rustc_middle::mir::{
88
self, Body, CallReturnPlaces, Location, SwitchTargetValue, TerminatorEdges,
99
};
10-
use rustc_middle::ty::util::Discr;
11-
use rustc_middle::ty::{self, TyCtxt};
12-
use smallvec::SmallVec;
10+
use rustc_middle::ty::{self, AdtDef, TyCtxt};
1311
use tracing::{debug, instrument};
1412

1513
use crate::drop_flag_effects::{DropFlagState, InactiveVariants};
@@ -22,30 +20,25 @@ use crate::{
2220
// Used by both `MaybeInitializedPlaces` and `MaybeUninitializedPlaces`.
2321
pub struct MaybePlacesSwitchIntData<'tcx> {
2422
enum_place: mir::Place<'tcx>,
25-
discriminants: Vec<(VariantIdx, Discr<'tcx>)>,
26-
index: usize,
23+
targets: Vec<(VariantIdx, mir::BasicBlock)>,
2724
}
2825

29-
impl<'tcx> MaybePlacesSwitchIntData<'tcx> {
30-
/// Creates a `SmallVec` mapping each target in `targets` to its `VariantIdx`.
31-
fn variants(&mut self, targets: &mir::SwitchTargets) -> SmallVec<[VariantIdx; 4]> {
32-
self.index = 0;
33-
targets.all_values().iter().map(|value| self.next_discr(value.get())).collect()
34-
}
35-
36-
// The discriminant order in the `SwitchInt` targets should match the order yielded by
37-
// `AdtDef::discriminants`. We rely on this to match each discriminant in the targets to its
38-
// corresponding variant in linear time.
39-
fn next_discr(&mut self, value: u128) -> VariantIdx {
40-
// An out-of-bounds abort will occur if the discriminant ordering isn't as described above.
41-
loop {
42-
let (variant, discr) = self.discriminants[self.index];
43-
self.index += 1;
44-
if discr.val == value {
45-
return variant;
46-
}
47-
}
48-
}
26+
/// Maps values of targets in `SwitchTargets` to `(VariantIdx, BasicBlock).` Panics if the variants
27+
/// in `targets` aren't in the same order as `AdtDef::discriminants`.
28+
fn collect_switch_targets<'tcx>(
29+
enum_def: AdtDef<'tcx>,
30+
targets: &mir::SwitchTargets,
31+
tcx: TyCtxt<'tcx>,
32+
) -> Vec<(VariantIdx, mir::BasicBlock)> {
33+
let mut discriminants = enum_def.discriminants(tcx);
34+
35+
Vec::from_iter(targets.iter().map(|(value, bb)| {
36+
let Some((variant_idx, _)) = discriminants.find(|(_, discr)| discr.val == value) else {
37+
bug!("ran out of discriminants before matching all switch targets");
38+
};
39+
40+
(variant_idx, bb)
41+
}))
4942
}
5043

5144
impl<'tcx> MaybePlacesSwitchIntData<'tcx> {
@@ -54,6 +47,7 @@ impl<'tcx> MaybePlacesSwitchIntData<'tcx> {
5447
body: &Body<'tcx>,
5548
block: mir::BasicBlock,
5649
discr: &mir::Operand<'tcx>,
50+
targets: &mir::SwitchTargets,
5751
) -> Option<Self> {
5852
let Some(discr) = discr.place() else { return None };
5953

@@ -78,8 +72,7 @@ impl<'tcx> MaybePlacesSwitchIntData<'tcx> {
7872
ty::Adt(enum_def, _) => {
7973
return Some(MaybePlacesSwitchIntData {
8074
enum_place,
81-
discriminants: enum_def.discriminants(tcx).collect(),
82-
index: 0,
75+
targets: collect_switch_targets(*enum_def, targets, tcx),
8376
});
8477
}
8578

@@ -448,25 +441,32 @@ impl<'tcx> Analysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> {
448441
&mut self,
449442
block: mir::BasicBlock,
450443
discr: &mir::Operand<'tcx>,
444+
targets: &mir::SwitchTargets,
451445
) -> Option<Self::SwitchIntData> {
452446
if !self.tcx.sess.opts.unstable_opts.precise_enum_drop_elaboration {
453447
return None;
454448
}
455449

456-
MaybePlacesSwitchIntData::new(self.tcx, self.body, block, discr)
450+
MaybePlacesSwitchIntData::new(self.tcx, self.body, block, discr, targets)
451+
}
452+
453+
#[inline]
454+
fn switch_int_target_variants<'a>(
455+
data: &'a Self::SwitchIntData,
456+
) -> impl Iterator<Item = &'a (VariantIdx, mir::BasicBlock)> {
457+
data.targets.iter()
457458
}
458459

459460
fn apply_switch_int_edge_effect(
460461
&mut self,
461-
data: &mut Self::SwitchIntData,
462+
data: &Self::SwitchIntData,
462463
state: &mut Self::Domain,
463464
value: SwitchTargetValue,
464-
targets: &mir::SwitchTargets,
465465
) {
466466
let inactive_variants = match value {
467-
SwitchTargetValue::Normal(value) => InactiveVariants::Active(data.next_discr(value)),
467+
SwitchTargetValue::Normal(variant_idx) => InactiveVariants::Active(variant_idx),
468468
SwitchTargetValue::Otherwise if self.exclude_inactive_in_otherwise => {
469-
InactiveVariants::Inactives(data.variants(targets))
469+
InactiveVariants::Inactives(&data.targets)
470470
}
471471
_ => return,
472472
};
@@ -564,6 +564,7 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
564564
&mut self,
565565
block: mir::BasicBlock,
566566
discr: &mir::Operand<'tcx>,
567+
targets: &mir::SwitchTargets,
567568
) -> Option<Self::SwitchIntData> {
568569
if !self.tcx.sess.opts.unstable_opts.precise_enum_drop_elaboration {
569570
return None;
@@ -573,20 +574,26 @@ impl<'tcx> Analysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> {
573574
return None;
574575
}
575576

576-
MaybePlacesSwitchIntData::new(self.tcx, self.body, block, discr)
577+
MaybePlacesSwitchIntData::new(self.tcx, self.body, block, discr, targets)
578+
}
579+
580+
#[inline]
581+
fn switch_int_target_variants<'a>(
582+
data: &'a Self::SwitchIntData,
583+
) -> impl Iterator<Item = &'a (VariantIdx, mir::BasicBlock)> {
584+
data.targets.iter()
577585
}
578586

579587
fn apply_switch_int_edge_effect(
580588
&mut self,
581-
data: &mut Self::SwitchIntData,
589+
data: &Self::SwitchIntData,
582590
state: &mut Self::Domain,
583591
value: SwitchTargetValue,
584-
targets: &mir::SwitchTargets,
585592
) {
586593
let inactive_variants = match value {
587-
SwitchTargetValue::Normal(value) => InactiveVariants::Active(data.next_discr(value)),
594+
SwitchTargetValue::Normal(variant_idx) => InactiveVariants::Active(variant_idx),
588595
SwitchTargetValue::Otherwise if self.include_inactive_in_otherwise => {
589-
InactiveVariants::Inactives(data.variants(targets))
596+
InactiveVariants::Inactives(&data.targets)
590597
}
591598
_ => return,
592599
};

0 commit comments

Comments
 (0)