Skip to content

Commit 272dd0b

Browse files
dianqkdianqk
committed
Revert "Auto merge of #120268 - DianQK:otherwise_is_last_variant_switchs, r=oli-obk"
This reverts commit 14fbc3c, reversing changes made to 9fb91aa.
1 parent a165f1f commit 272dd0b

File tree

38 files changed

+155
-1396
lines changed

38 files changed

+155
-1396
lines changed

‎compiler/rustc_middle/src/mir/patch.rs

+2-25
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ pub struct MirPatch<'tcx> {
1111
resume_block: Option<BasicBlock>,
1212
// Only for unreachable in cleanup path.
1313
unreachable_cleanup_block: Option<BasicBlock>,
14-
// Only for unreachable not in cleanup path.
15-
unreachable_no_cleanup_block: Option<BasicBlock>,
1614
// Cached block for UnwindTerminate (with reason)
1715
terminate_block: Option<(BasicBlock, UnwindTerminateReason)>,
1816
body_span: Span,
@@ -29,7 +27,6 @@ impl<'tcx> MirPatch<'tcx> {
2927
next_local: body.local_decls.len(),
3028
resume_block: None,
3129
unreachable_cleanup_block: None,
32-
unreachable_no_cleanup_block: None,
3330
terminate_block: None,
3431
body_span: body.span,
3532
};
@@ -46,12 +43,9 @@ impl<'tcx> MirPatch<'tcx> {
4643
// Check if we already have an unreachable block
4744
if matches!(block.terminator().kind, TerminatorKind::Unreachable)
4845
&& block.statements.is_empty()
46+
&& block.is_cleanup
4947
{
50-
if block.is_cleanup {
51-
result.unreachable_cleanup_block = Some(bb);
52-
} else {
53-
result.unreachable_no_cleanup_block = Some(bb);
54-
}
48+
result.unreachable_cleanup_block = Some(bb);
5549
continue;
5650
}
5751

@@ -101,23 +95,6 @@ impl<'tcx> MirPatch<'tcx> {
10195
bb
10296
}
10397

104-
pub fn unreachable_no_cleanup_block(&mut self) -> BasicBlock {
105-
if let Some(bb) = self.unreachable_no_cleanup_block {
106-
return bb;
107-
}
108-
109-
let bb = self.new_block(BasicBlockData {
110-
statements: vec![],
111-
terminator: Some(Terminator {
112-
source_info: SourceInfo::outermost(self.body_span),
113-
kind: TerminatorKind::Unreachable,
114-
}),
115-
is_cleanup: false,
116-
});
117-
self.unreachable_no_cleanup_block = Some(bb);
118-
bb
119-
}
120-
12198
pub fn terminate_block(&mut self, reason: UnwindTerminateReason) -> BasicBlock {
12299
if let Some((cached_bb, cached_reason)) = self.terminate_block
123100
&& reason == cached_reason

‎compiler/rustc_middle/src/mir/terminator.rs

-11
Original file line numberDiff line numberDiff line change
@@ -74,17 +74,6 @@ impl SwitchTargets {
7474
pub fn target_for_value(&self, value: u128) -> BasicBlock {
7575
self.iter().find_map(|(v, t)| (v == value).then_some(t)).unwrap_or_else(|| self.otherwise())
7676
}
77-
78-
/// Adds a new target to the switch. But You cannot add an already present value.
79-
#[inline]
80-
pub fn add_target(&mut self, value: u128, bb: BasicBlock) {
81-
let value = Pu128(value);
82-
if self.values.contains(&value) {
83-
bug!("target value {:?} already present", value);
84-
}
85-
self.values.push(value);
86-
self.targets.insert(self.targets.len() - 1, bb);
87-
}
8877
}
8978

9079
pub struct SwitchTargetsIter<'a> {

‎compiler/rustc_mir_transform/src/uninhabited_enum_branching.rs

+27-57
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22
33
use crate::MirPass;
44
use rustc_data_structures::fx::FxHashSet;
5-
use rustc_middle::mir::patch::MirPatch;
65
use rustc_middle::mir::{
7-
BasicBlock, BasicBlockData, BasicBlocks, Body, Local, Operand, Rvalue, StatementKind,
8-
TerminatorKind,
6+
BasicBlockData, Body, Local, Operand, Rvalue, StatementKind, Terminator, TerminatorKind,
97
};
108
use rustc_middle::ty::layout::TyAndLayout;
119
use rustc_middle::ty::{Ty, TyCtxt};
@@ -79,8 +77,7 @@ impl<'tcx> MirPass<'tcx> for UninhabitedEnumBranching {
7977
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
8078
trace!("UninhabitedEnumBranching starting for {:?}", body.source);
8179

82-
let mut unreachable_targets = Vec::new();
83-
let mut patch = MirPatch::new(body);
80+
let mut removable_switchs = Vec::new();
8481

8582
for (bb, bb_data) in body.basic_blocks.iter_enumerated() {
8683
trace!("processing block {:?}", bb);
@@ -95,73 +92,46 @@ impl<'tcx> MirPass<'tcx> for UninhabitedEnumBranching {
9592
tcx.param_env_reveal_all_normalized(body.source.def_id()).and(discriminant_ty),
9693
);
9794

98-
let mut allowed_variants = if let Ok(layout) = layout {
95+
let allowed_variants = if let Ok(layout) = layout {
9996
variant_discriminants(&layout, discriminant_ty, tcx)
100-
} else if let Some(variant_range) = discriminant_ty.variant_range(tcx) {
101-
variant_range
102-
.map(|variant| {
103-
discriminant_ty.discriminant_for_variant(tcx, variant).unwrap().val
104-
})
105-
.collect()
10697
} else {
10798
continue;
10899
};
109100

110101
trace!("allowed_variants = {:?}", allowed_variants);
111102

112-
unreachable_targets.clear();
113-
let TerminatorKind::SwitchInt { targets, discr } = &bb_data.terminator().kind else {
114-
bug!()
115-
};
103+
let terminator = bb_data.terminator();
104+
let TerminatorKind::SwitchInt { targets, .. } = &terminator.kind else { bug!() };
116105

106+
let mut reachable_count = 0;
117107
for (index, (val, _)) in targets.iter().enumerate() {
118-
if !allowed_variants.remove(&val) {
119-
unreachable_targets.push(index);
120-
}
121-
}
122-
let otherwise_is_empty_unreachable =
123-
body.basic_blocks[targets.otherwise()].is_empty_unreachable();
124-
// After resolving https://github.com/llvm/llvm-project/issues/78578,
125-
// we can remove the limit on the number of successors.
126-
fn check_successors(basic_blocks: &BasicBlocks<'_>, bb: BasicBlock) -> bool {
127-
let mut successors = basic_blocks[bb].terminator().successors();
128-
let Some(first_successor) = successors.next() else { return true };
129-
if successors.next().is_some() {
130-
return true;
108+
if allowed_variants.contains(&val) {
109+
reachable_count += 1;
110+
} else {
111+
removable_switchs.push((bb, index));
131112
}
132-
if let TerminatorKind::SwitchInt { .. } =
133-
&basic_blocks[first_successor].terminator().kind
134-
{
135-
return false;
136-
};
137-
true
138113
}
139-
let otherwise_is_last_variant = !otherwise_is_empty_unreachable
140-
&& allowed_variants.len() == 1
141-
&& check_successors(&body.basic_blocks, targets.otherwise());
142-
let replace_otherwise_to_unreachable = otherwise_is_last_variant
143-
|| !otherwise_is_empty_unreachable && allowed_variants.is_empty();
144114

145-
if unreachable_targets.is_empty() && !replace_otherwise_to_unreachable {
146-
continue;
115+
if reachable_count == allowed_variants.len() {
116+
removable_switchs.push((bb, targets.iter().count()));
147117
}
118+
}
148119

149-
let unreachable_block = patch.unreachable_no_cleanup_block();
150-
let mut targets = targets.clone();
151-
if replace_otherwise_to_unreachable {
152-
if otherwise_is_last_variant {
153-
#[allow(rustc::potential_query_instability)]
154-
let last_variant = *allowed_variants.iter().next().unwrap();
155-
targets.add_target(last_variant, targets.otherwise());
156-
}
157-
unreachable_targets.push(targets.iter().count());
158-
}
159-
for index in unreachable_targets.iter() {
160-
targets.all_targets_mut()[*index] = unreachable_block;
161-
}
162-
patch.patch_terminator(bb, TerminatorKind::SwitchInt { targets, discr: discr.clone() });
120+
if removable_switchs.is_empty() {
121+
return;
163122
}
164123

165-
patch.apply(body);
124+
let new_block = BasicBlockData::new(Some(Terminator {
125+
source_info: body.basic_blocks[removable_switchs[0].0].terminator().source_info,
126+
kind: TerminatorKind::Unreachable,
127+
}));
128+
let unreachable_block = body.basic_blocks.as_mut().push(new_block);
129+
130+
for (bb, index) in removable_switchs {
131+
let bb = &mut body.basic_blocks.as_mut()[bb];
132+
let terminator = bb.terminator_mut();
133+
let TerminatorKind::SwitchInt { targets, .. } = &mut terminator.kind else { bug!() };
134+
targets.all_targets_mut()[index] = unreachable_block;
135+
}
166136
}
167137
}

‎tests/codegen/enum/uninhabited_enum_default_branch.rs

-24
This file was deleted.

‎tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff

+1-5
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
StorageLive(_6);
7070
_6 = ((*_1).4: std::option::Option<usize>);
7171
_7 = discriminant(_6);
72-
switchInt(move _7) -> [1: bb4, 0: bb6, otherwise: bb9];
72+
switchInt(move _7) -> [1: bb4, otherwise: bb6];
7373
}
7474

7575
bb4: {
@@ -135,9 +135,5 @@
135135
StorageDead(_6);
136136
return;
137137
}
138-
139-
bb9: {
140-
unreachable;
141-
}
142138
}
143139

‎tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-unwind.diff

+1-5
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
StorageLive(_6);
7070
_6 = ((*_1).4: std::option::Option<usize>);
7171
_7 = discriminant(_6);
72-
switchInt(move _7) -> [1: bb4, 0: bb6, otherwise: bb9];
72+
switchInt(move _7) -> [1: bb4, otherwise: bb6];
7373
}
7474

7575
bb4: {
@@ -135,9 +135,5 @@
135135
StorageDead(_6);
136136
return;
137137
}
138-
139-
bb9: {
140-
unreachable;
141-
}
142138
}
143139

‎tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-abort.mir

+10-12
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,21 @@ fn num_to_digit(_1: char) -> u32 {
3333
_3 = &_2;
3434
StorageLive(_4);
3535
_4 = discriminant(_2);
36-
switchInt(move _4) -> [1: bb2, 0: bb6, otherwise: bb8];
36+
StorageDead(_3);
37+
StorageDead(_2);
38+
switchInt(move _4) -> [1: bb2, otherwise: bb7];
3739
}
3840

3941
bb2: {
4042
StorageDead(_4);
41-
StorageDead(_3);
42-
StorageDead(_2);
4343
StorageLive(_5);
4444
_5 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind unreachable];
4545
}
4646

4747
bb3: {
4848
StorageLive(_6);
4949
_6 = discriminant(_5);
50-
switchInt(move _6) -> [0: bb4, 1: bb5, otherwise: bb8];
50+
switchInt(move _6) -> [0: bb4, 1: bb5, otherwise: bb6];
5151
}
5252

5353
bb4: {
@@ -58,22 +58,20 @@ fn num_to_digit(_1: char) -> u32 {
5858
_0 = move ((_5 as Some).0: u32);
5959
StorageDead(_6);
6060
StorageDead(_5);
61-
goto -> bb7;
61+
goto -> bb8;
6262
}
6363

6464
bb6: {
65-
StorageDead(_4);
66-
StorageDead(_3);
67-
StorageDead(_2);
68-
_0 = const 0_u32;
69-
goto -> bb7;
65+
unreachable;
7066
}
7167

7268
bb7: {
73-
return;
69+
StorageDead(_4);
70+
_0 = const 0_u32;
71+
goto -> bb8;
7472
}
7573

7674
bb8: {
77-
unreachable;
75+
return;
7876
}
7977
}

‎tests/mir-opt/issues/issue_59352.num_to_digit.PreCodegen.after.panic-unwind.mir

+10-12
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,21 @@ fn num_to_digit(_1: char) -> u32 {
3333
_3 = &_2;
3434
StorageLive(_4);
3535
_4 = discriminant(_2);
36-
switchInt(move _4) -> [1: bb2, 0: bb6, otherwise: bb8];
36+
StorageDead(_3);
37+
StorageDead(_2);
38+
switchInt(move _4) -> [1: bb2, otherwise: bb7];
3739
}
3840

3941
bb2: {
4042
StorageDead(_4);
41-
StorageDead(_3);
42-
StorageDead(_2);
4343
StorageLive(_5);
4444
_5 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind continue];
4545
}
4646

4747
bb3: {
4848
StorageLive(_6);
4949
_6 = discriminant(_5);
50-
switchInt(move _6) -> [0: bb4, 1: bb5, otherwise: bb8];
50+
switchInt(move _6) -> [0: bb4, 1: bb5, otherwise: bb6];
5151
}
5252

5353
bb4: {
@@ -58,22 +58,20 @@ fn num_to_digit(_1: char) -> u32 {
5858
_0 = move ((_5 as Some).0: u32);
5959
StorageDead(_6);
6060
StorageDead(_5);
61-
goto -> bb7;
61+
goto -> bb8;
6262
}
6363

6464
bb6: {
65-
StorageDead(_4);
66-
StorageDead(_3);
67-
StorageDead(_2);
68-
_0 = const 0_u32;
69-
goto -> bb7;
65+
unreachable;
7066
}
7167

7268
bb7: {
73-
return;
69+
StorageDead(_4);
70+
_0 = const 0_u32;
71+
goto -> bb8;
7472
}
7573

7674
bb8: {
77-
unreachable;
75+
return;
7876
}
7977
}

0 commit comments

Comments
 (0)