Skip to content

Commit 930898b

Browse files
committed
Do not invent new basic block - just jump to already existing bb
Fixes a miscompilation and generally simplifies the MIR generated
1 parent 3d52e54 commit 930898b

11 files changed

+358
-267
lines changed

compiler/rustc_mir/src/transform/early_otherwise_branch.rs

+12-25
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
};
55
use rustc_middle::mir::*;
66
use rustc_middle::ty::{Ty, TyCtxt};
7-
use std::{borrow::Cow, fmt::Debug};
7+
use std::fmt::Debug;
88

99
use super::simplify::simplify_cfg;
1010

@@ -98,33 +98,20 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
9898
StatementKind::Assign(box (Place::from(not_equal_temp), not_equal_rvalue)),
9999
);
100100

101-
let (mut targets_to_jump_to, values_to_jump_to): (Vec<_>, Vec<_>) = opt_to_apply
102-
.infos
103-
.iter()
104-
.flat_map(|x| x.second_switch_info.targets_with_values.iter())
105-
.cloned()
106-
.unzip();
107-
108-
// add otherwise case in the end
109-
targets_to_jump_to.push(opt_to_apply.infos[0].first_switch_info.otherwise_bb);
110-
// new block that jumps to the correct discriminant case. This block is switched to if the discriminants are equal
111-
let new_switch_data = BasicBlockData::new(Some(Terminator {
112-
source_info: opt_to_apply.infos[0].second_switch_info.discr_source_info,
113-
kind: TerminatorKind::SwitchInt {
114-
// the first and second discriminants are equal, so just pick one
115-
discr: Operand::Copy(first_descriminant_place),
116-
switch_ty: discr_type,
117-
values: Cow::from(values_to_jump_to),
118-
targets: targets_to_jump_to,
119-
},
120-
}));
121-
122-
let new_switch_bb = patch.new_block(new_switch_data);
123-
124101
// switch on the NotEqual. If true, then jump to the `otherwise` case.
125102
// If false, then jump to a basic block that then jumps to the correct disciminant case
126103
let true_case = opt_to_apply.infos[0].first_switch_info.otherwise_bb;
127-
let false_case = new_switch_bb;
104+
let targets_second_switch =
105+
&opt_to_apply.infos[0].second_switch_info.targets_with_values;
106+
assert_eq!(
107+
1,
108+
targets_second_switch.len(),
109+
"We should only have one target besides the otherwise"
110+
);
111+
112+
// Since we know that the two discriminant values are equal,
113+
// we can jump directly to the target in the second switch
114+
let false_case = targets_second_switch[0].0;
128115
patch.patch_terminator(
129116
opt_to_apply.basic_block_first_switch,
130117
TerminatorKind::if_(

src/test/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff

+2-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
+ StorageLive(_11); // scope 0 at $DIR/early_otherwise_branch.rs:5:10: 5:17
3737
+ _11 = Ne(_10, _7); // scope 0 at $DIR/early_otherwise_branch.rs:5:10: 5:17
3838
+ StorageDead(_10); // scope 0 at $DIR/early_otherwise_branch.rs:5:10: 5:17
39-
+ switchInt(move _11) -> [false: bb4, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:5:10: 5:17
39+
+ switchInt(move _11) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:5:10: 5:17
4040
}
4141

4242
bb1: {
@@ -52,6 +52,7 @@
5252
- }
5353
-
5454
- bb3: {
55+
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:5:15: 5:16
5556
StorageLive(_8); // scope 0 at $DIR/early_otherwise_branch.rs:5:15: 5:16
5657
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:5:15: 5:16
5758
StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:5:24: 5:25
@@ -67,11 +68,6 @@
6768
+ bb3: {
6869
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:8:1: 8:2
6970
return; // scope 0 at $DIR/early_otherwise_branch.rs:8:2: 8:2
70-
+ }
71-
+
72-
+ bb4: {
73-
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:5:19: 5:26
74-
+ switchInt(_7) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:5:19: 5:26
7571
}
7672
}
7773

src/test/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff

+15-21
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
+ StorageLive(_12); // scope 0 at $DIR/early_otherwise_branch.rs:13:10: 13:17
3838
+ _12 = Ne(_11, _8); // scope 0 at $DIR/early_otherwise_branch.rs:13:10: 13:17
3939
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:13:10: 13:17
40-
+ switchInt(move _12) -> [false: bb5, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:13:10: 13:17
40+
+ switchInt(move _12) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:13:10: 13:17
4141
}
4242

4343
bb1: {
@@ -49,7 +49,7 @@
4949
+ StorageDead(_12); // scope 0 at $DIR/early_otherwise_branch.rs:15:14: 15:15
5050
_0 = const 1_u32; // scope 0 at $DIR/early_otherwise_branch.rs:15:14: 15:15
5151
- goto -> bb6; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
52-
+ goto -> bb4; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
52+
+ goto -> bb3; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
5353
}
5454

5555
- bb3: {
@@ -58,34 +58,28 @@
5858
- }
5959
-
6060
- bb4: {
61-
+ bb2: {
62-
StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:13:15: 13:16
63-
_9 = (((_3.0: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:13:15: 13:16
64-
StorageLive(_10); // scope 0 at $DIR/early_otherwise_branch.rs:13:24: 13:25
65-
_10 = (((_3.1: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:13:24: 13:25
66-
_0 = const 0_u32; // scope 1 at $DIR/early_otherwise_branch.rs:13:31: 13:32
67-
StorageDead(_10); // scope 0 at $DIR/early_otherwise_branch.rs:13:31: 13:32
68-
StorageDead(_9); // scope 0 at $DIR/early_otherwise_branch.rs:13:31: 13:32
61+
- StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:13:15: 13:16
62+
- _9 = (((_3.0: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:13:15: 13:16
63+
- StorageLive(_10); // scope 0 at $DIR/early_otherwise_branch.rs:13:24: 13:25
64+
- _10 = (((_3.1: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:13:24: 13:25
65+
- _0 = const 0_u32; // scope 1 at $DIR/early_otherwise_branch.rs:13:31: 13:32
66+
- StorageDead(_10); // scope 0 at $DIR/early_otherwise_branch.rs:13:31: 13:32
67+
- StorageDead(_9); // scope 0 at $DIR/early_otherwise_branch.rs:13:31: 13:32
6968
- goto -> bb6; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
70-
+ goto -> bb4; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
71-
}
72-
69+
- }
70+
-
7371
- bb5: {
74-
+ bb3: {
72+
+ bb2: {
73+
+ StorageDead(_12); // scope 0 at $DIR/early_otherwise_branch.rs:14:25: 14:26
7574
_0 = const 0_u32; // scope 0 at $DIR/early_otherwise_branch.rs:14:25: 14:26
7675
- goto -> bb6; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
77-
+ goto -> bb4; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
76+
+ goto -> bb3; // scope 0 at $DIR/early_otherwise_branch.rs:12:5: 16:6
7877
}
7978

8079
- bb6: {
81-
+ bb4: {
80+
+ bb3: {
8281
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:17:1: 17:2
8382
return; // scope 0 at $DIR/early_otherwise_branch.rs:17:2: 17:2
84-
+ }
85-
+
86-
+ bb5: {
87-
+ StorageDead(_12); // scope 0 at $DIR/early_otherwise_branch.rs:14:16: 14:20
88-
+ switchInt(_8) -> [0_isize: bb3, 1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:14:16: 14:20
8983
}
9084
}
9185

src/test/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff

+2-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
+ StorageLive(_11); // scope 0 at $DIR/early_otherwise_branch.rs:33:10: 33:28
3737
+ _11 = Ne(_10, _7); // scope 0 at $DIR/early_otherwise_branch.rs:33:10: 33:28
3838
+ StorageDead(_10); // scope 0 at $DIR/early_otherwise_branch.rs:33:10: 33:28
39-
+ switchInt(move _11) -> [false: bb4, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:33:10: 33:28
39+
+ switchInt(move _11) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:33:10: 33:28
4040
}
4141

4242
bb1: {
@@ -53,6 +53,7 @@
5353

5454
- bb3: {
5555
+ bb2: {
56+
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:33:26: 33:27
5657
StorageLive(_8); // scope 0 at $DIR/early_otherwise_branch.rs:33:26: 33:27
5758
_8 = (((_3.0: MyOption1<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:33:26: 33:27
5859
StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:33:46: 33:47
@@ -68,11 +69,6 @@
6869
+ bb3: {
6970
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:36:1: 36:2
7071
return; // scope 0 at $DIR/early_otherwise_branch.rs:36:2: 36:2
71-
+ }
72-
+
73-
+ bb4: {
74-
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:33:30: 33:48
75-
+ switchInt(_7) -> [0_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:33:30: 33:48
7672
}
7773
}
7874

src/test/mir-opt/early_otherwise_branch.opt4.EarlyOtherwiseBranch.diff

+11-14
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
+ StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:42:10: 42:16
3131
+ _9 = Ne(_8, _7); // scope 0 at $DIR/early_otherwise_branch.rs:42:10: 42:16
3232
+ StorageDead(_8); // scope 0 at $DIR/early_otherwise_branch.rs:42:10: 42:16
33-
+ switchInt(move _9) -> [false: bb4, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:42:10: 42:16
33+
+ switchInt(move _9) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:42:10: 42:16
3434
}
3535

3636
bb1: {
@@ -43,22 +43,19 @@
4343
bb2: {
4444
- _6 = discriminant((_3.1: std::option::Option<u32>)); // scope 0 at $DIR/early_otherwise_branch.rs:42:18: 42:25
4545
- switchInt(move _6) -> [1_isize: bb3, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:42:18: 42:25
46-
+ _0 = const 0_u32; // scope 0 at $DIR/early_otherwise_branch.rs:42:30: 42:31
47-
+ goto -> bb3; // scope 0 at $DIR/early_otherwise_branch.rs:41:5: 44:6
48-
}
49-
50-
bb3: {
51-
- _0 = const 0_u32; // scope 0 at $DIR/early_otherwise_branch.rs:42:30: 42:31
46+
- }
47+
-
48+
- bb3: {
49+
+ StorageDead(_9); // scope 0 at $DIR/early_otherwise_branch.rs:42:30: 42:31
50+
_0 = const 0_u32; // scope 0 at $DIR/early_otherwise_branch.rs:42:30: 42:31
5251
- goto -> bb4; // scope 0 at $DIR/early_otherwise_branch.rs:41:5: 44:6
53-
+ StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:45:1: 45:2
54-
+ return; // scope 0 at $DIR/early_otherwise_branch.rs:45:2: 45:2
52+
+ goto -> bb3; // scope 0 at $DIR/early_otherwise_branch.rs:41:5: 44:6
5553
}
5654

57-
bb4: {
58-
- StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:45:1: 45:2
59-
- return; // scope 0 at $DIR/early_otherwise_branch.rs:45:2: 45:2
60-
+ StorageDead(_9); // scope 0 at $DIR/early_otherwise_branch.rs:42:18: 42:25
61-
+ switchInt(_7) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:42:18: 42:25
55+
- bb4: {
56+
+ bb3: {
57+
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:45:1: 45:2
58+
return; // scope 0 at $DIR/early_otherwise_branch.rs:45:2: 45:2
6259
}
6360
}
6461

src/test/mir-opt/early_otherwise_branch.opt5.EarlyOtherwiseBranch.diff

+2-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
+ StorageLive(_11); // scope 0 at $DIR/early_otherwise_branch.rs:51:10: 51:17
3737
+ _11 = Ne(_10, _7); // scope 0 at $DIR/early_otherwise_branch.rs:51:10: 51:17
3838
+ StorageDead(_10); // scope 0 at $DIR/early_otherwise_branch.rs:51:10: 51:17
39-
+ switchInt(move _11) -> [false: bb4, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:51:10: 51:17
39+
+ switchInt(move _11) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:51:10: 51:17
4040
}
4141

4242
bb1: {
@@ -52,6 +52,7 @@
5252
- }
5353
-
5454
- bb3: {
55+
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:51:15: 51:16
5556
StorageLive(_8); // scope 0 at $DIR/early_otherwise_branch.rs:51:15: 51:16
5657
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32); // scope 0 at $DIR/early_otherwise_branch.rs:51:15: 51:16
5758
StorageLive(_9); // scope 0 at $DIR/early_otherwise_branch.rs:51:24: 51:25
@@ -67,11 +68,6 @@
6768
+ bb3: {
6869
StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch.rs:54:1: 54:2
6970
return; // scope 0 at $DIR/early_otherwise_branch.rs:54:2: 54:2
70-
+ }
71-
+
72-
+ bb4: {
73-
+ StorageDead(_11); // scope 0 at $DIR/early_otherwise_branch.rs:51:19: 51:26
74-
+ switchInt(_7) -> [1_isize: bb2, otherwise: bb1]; // scope 0 at $DIR/early_otherwise_branch.rs:51:19: 51:26
7571
}
7672
}
7773

0 commit comments

Comments
 (0)