Skip to content

Commit 7a8b62c

Browse files
committed
Auto merge of rust-lang#121397 - DianQK:early_otherwise_branch_sound, r=<try>
Re-enable the early otherwise branch optimization Fixes rust-lang#95162. Fixes rust-lang#119014. Fixes rust-lang#117970. An invalid enum discriminant can come from anywhere. We have to check to see if all successors contain the discriminant statement. It should not be UB that we pass in an invalid enum discriminant when calling a function, this is more like LLVM's poison value. UB only when used. Although miri would consider the following code to be UB. (It's fine for miri.) https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=18602870aaeb07cbdf7dfcd2c28961a2 I extended the scenario with scalars and the same target values. r? compiler
2 parents d3d145e + d5c6f73 commit 7a8b62c

20 files changed

+1131
-233
lines changed

compiler/rustc_mir_transform/src/early_otherwise_branch.rs

+220-130
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//@ compile-flags: -O
2+
3+
#![crate_type = "lib"]
4+
5+
pub enum Enum {
6+
A(u32),
7+
B(u32),
8+
C(u32),
9+
}
10+
11+
#[no_mangle]
12+
pub fn foo(lhs: &Enum, rhs: &Enum) -> bool {
13+
// CHECK-LABEL: define{{.*}}i1 @foo(
14+
// CHECK-NOT: switch
15+
// CHECK-NOT: br
16+
// CHECK: [[SELECT:%.*]] = select
17+
// CHECK-NEXT: ret i1 [[SELECT]]
18+
// CHECK-NEXT: }
19+
match (lhs, rhs) {
20+
(Enum::A(lhs), Enum::A(rhs)) => lhs == rhs,
21+
(Enum::B(lhs), Enum::B(rhs)) => lhs == rhs,
22+
(Enum::C(lhs), Enum::C(rhs)) => lhs == rhs,
23+
_ => false,
24+
}
25+
}

tests/mir-opt/early_otherwise_branch.opt1.EarlyOtherwiseBranch.diff

+9-26
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
let mut _7: isize;
1313
let _8: u32;
1414
let _9: u32;
15-
+ let mut _10: isize;
16-
+ let mut _11: bool;
1715
scope 1 {
1816
debug a => _8;
1917
debug b => _9;
@@ -29,48 +27,33 @@
2927
StorageDead(_5);
3028
StorageDead(_4);
3129
_7 = discriminant((_3.0: std::option::Option<u32>));
32-
- switchInt(move _7) -> [1: bb2, otherwise: bb1];
33-
+ StorageLive(_10);
34-
+ _10 = discriminant((_3.1: std::option::Option<u32>));
35-
+ StorageLive(_11);
36-
+ _11 = Ne(_7, move _10);
37-
+ StorageDead(_10);
38-
+ switchInt(move _11) -> [0: bb4, otherwise: bb1];
30+
switchInt(move _7) -> [1: bb2, otherwise: bb1];
3931
}
4032

4133
bb1: {
42-
+ StorageDead(_11);
4334
_0 = const 1_u32;
44-
- goto -> bb4;
45-
+ goto -> bb3;
35+
goto -> bb4;
4636
}
4737

4838
bb2: {
49-
- _6 = discriminant((_3.1: std::option::Option<u32>));
50-
- switchInt(move _6) -> [1: bb3, otherwise: bb1];
51-
- }
52-
-
53-
- bb3: {
39+
_6 = discriminant((_3.1: std::option::Option<u32>));
40+
switchInt(move _6) -> [1: bb3, otherwise: bb1];
41+
}
42+
43+
bb3: {
5444
StorageLive(_9);
5545
_9 = (((_3.1: std::option::Option<u32>) as Some).0: u32);
5646
StorageLive(_8);
5747
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
5848
_0 = const 0_u32;
5949
StorageDead(_8);
6050
StorageDead(_9);
61-
- goto -> bb4;
62-
+ goto -> bb3;
51+
goto -> bb4;
6352
}
6453

65-
- bb4: {
66-
+ bb3: {
54+
bb4: {
6755
StorageDead(_3);
6856
return;
69-
+ }
70-
+
71-
+ bb4: {
72-
+ StorageDead(_11);
73-
+ switchInt(_7) -> [1: bb2, otherwise: bb1];
7457
}
7558
}
7659

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
- // MIR for `opt10` before EarlyOtherwiseBranch
2+
+ // MIR for `opt10` after EarlyOtherwiseBranch
3+
4+
fn opt10(_1: E8, _2: E16) -> u32 {
5+
debug x => _1;
6+
debug y => _2;
7+
let mut _0: u32;
8+
let mut _3: (E8, E16);
9+
let mut _4: E8;
10+
let mut _5: E16;
11+
let mut _6: u16;
12+
let mut _7: u16;
13+
let mut _8: u16;
14+
let mut _9: u8;
15+
+ let mut _10: u16;
16+
17+
bb0: {
18+
StorageLive(_3);
19+
StorageLive(_4);
20+
_4 = move _1;
21+
StorageLive(_5);
22+
_5 = move _2;
23+
_3 = (move _4, move _5);
24+
StorageDead(_5);
25+
StorageDead(_4);
26+
_9 = discriminant((_3.0: E8));
27+
- switchInt(move _9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb9];
28+
+ StorageLive(_10);
29+
+ _10 = discriminant((_3.1: E16));
30+
+ switchInt(move _10) -> [0: bb7, otherwise: bb1];
31+
}
32+
33+
bb1: {
34+
+ StorageDead(_10);
35+
_0 = const 0_u32;
36+
- goto -> bb8;
37+
+ goto -> bb5;
38+
}
39+
40+
bb2: {
41+
- _6 = discriminant((_3.1: E16));
42+
- switchInt(move _6) -> [0: bb5, otherwise: bb1];
43+
+ _0 = const 1_u32;
44+
+ goto -> bb5;
45+
}
46+
47+
bb3: {
48+
- _7 = discriminant((_3.1: E16));
49+
- switchInt(move _7) -> [0: bb6, otherwise: bb1];
50+
+ _0 = const 2_u32;
51+
+ goto -> bb5;
52+
}
53+
54+
bb4: {
55+
- _8 = discriminant((_3.1: E16));
56+
- switchInt(move _8) -> [0: bb7, otherwise: bb1];
57+
+ _0 = const 3_u32;
58+
+ goto -> bb5;
59+
}
60+
61+
bb5: {
62+
- _0 = const 1_u32;
63+
- goto -> bb8;
64+
+ StorageDead(_3);
65+
+ return;
66+
}
67+
68+
bb6: {
69+
- _0 = const 2_u32;
70+
- goto -> bb8;
71+
+ unreachable;
72+
}
73+
74+
bb7: {
75+
- _0 = const 3_u32;
76+
- goto -> bb8;
77+
- }
78+
-
79+
- bb8: {
80+
- StorageDead(_3);
81+
- return;
82+
- }
83+
-
84+
- bb9: {
85+
- unreachable;
86+
+ StorageDead(_10);
87+
+ switchInt(_9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb6];
88+
}
89+
}
90+

tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff

+10-5
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@
3030
StorageDead(_5);
3131
StorageDead(_4);
3232
_8 = discriminant((_3.0: std::option::Option<u32>));
33-
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb1];
33+
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb7];
3434
+ StorageLive(_11);
3535
+ _11 = discriminant((_3.1: std::option::Option<u32>));
3636
+ StorageLive(_12);
3737
+ _12 = Ne(_8, move _11);
3838
+ StorageDead(_11);
39-
+ switchInt(move _12) -> [0: bb5, otherwise: bb1];
39+
+ switchInt(move _12) -> [0: bb6, otherwise: bb1];
4040
}
4141

4242
bb1: {
@@ -70,7 +70,7 @@
7070

7171
- bb5: {
7272
+ bb3: {
73-
_0 = const 0_u32;
73+
_0 = const 2_u32;
7474
- goto -> bb6;
7575
+ goto -> bb4;
7676
}
@@ -79,11 +79,16 @@
7979
+ bb4: {
8080
StorageDead(_3);
8181
return;
82+
}
83+
84+
- bb7: {
85+
+ bb5: {
86+
unreachable;
8287
+ }
8388
+
84-
+ bb5: {
89+
+ bb6: {
8590
+ StorageDead(_12);
86-
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb1];
91+
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb5];
8792
}
8893
}
8994

tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff

+45-27
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@
1010
let mut _5: std::option::Option<bool>;
1111
let mut _6: isize;
1212
let mut _7: isize;
13-
let _8: u32;
14-
let _9: bool;
15-
+ let mut _10: isize;
16-
+ let mut _11: bool;
13+
let mut _8: isize;
14+
let _9: u32;
15+
let _10: bool;
16+
+ let mut _11: isize;
17+
+ let mut _12: bool;
1718
scope 1 {
18-
debug a => _8;
19-
debug b => _9;
19+
debug a => _9;
20+
debug b => _10;
2021
}
2122

2223
bb0: {
@@ -28,49 +29,66 @@
2829
_3 = (move _4, move _5);
2930
StorageDead(_5);
3031
StorageDead(_4);
31-
_7 = discriminant((_3.0: std::option::Option<u32>));
32-
- switchInt(move _7) -> [1: bb2, otherwise: bb1];
33-
+ StorageLive(_10);
34-
+ _10 = discriminant((_3.1: std::option::Option<bool>));
32+
_8 = discriminant((_3.0: std::option::Option<u32>));
33+
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb7];
3534
+ StorageLive(_11);
36-
+ _11 = Ne(_7, move _10);
37-
+ StorageDead(_10);
38-
+ switchInt(move _11) -> [0: bb4, otherwise: bb1];
35+
+ _11 = discriminant((_3.1: std::option::Option<bool>));
36+
+ StorageLive(_12);
37+
+ _12 = Ne(_8, move _11);
38+
+ StorageDead(_11);
39+
+ switchInt(move _12) -> [0: bb6, otherwise: bb1];
3940
}
4041

4142
bb1: {
42-
+ StorageDead(_11);
43+
+ StorageDead(_12);
4344
_0 = const 1_u32;
44-
- goto -> bb4;
45-
+ goto -> bb3;
45+
- goto -> bb6;
46+
+ goto -> bb4;
4647
}
4748

4849
bb2: {
4950
- _6 = discriminant((_3.1: std::option::Option<bool>));
50-
- switchInt(move _6) -> [1: bb3, otherwise: bb1];
51+
- switchInt(move _6) -> [0: bb5, otherwise: bb1];
5152
- }
5253
-
5354
- bb3: {
55+
- _7 = discriminant((_3.1: std::option::Option<bool>));
56+
- switchInt(move _7) -> [1: bb4, otherwise: bb1];
57+
- }
58+
-
59+
- bb4: {
60+
StorageLive(_10);
61+
_10 = (((_3.1: std::option::Option<bool>) as Some).0: bool);
5462
StorageLive(_9);
55-
_9 = (((_3.1: std::option::Option<bool>) as Some).0: bool);
56-
StorageLive(_8);
57-
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
63+
_9 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
5864
_0 = const 0_u32;
59-
StorageDead(_8);
6065
StorageDead(_9);
61-
- goto -> bb4;
62-
+ goto -> bb3;
66+
StorageDead(_10);
67+
- goto -> bb6;
68+
+ goto -> bb4;
6369
}
6470

65-
- bb4: {
71+
- bb5: {
6672
+ bb3: {
73+
_0 = const 2_u32;
74+
- goto -> bb6;
75+
+ goto -> bb4;
76+
}
77+
78+
- bb6: {
79+
+ bb4: {
6780
StorageDead(_3);
6881
return;
82+
}
83+
84+
- bb7: {
85+
+ bb5: {
86+
unreachable;
6987
+ }
7088
+
71-
+ bb4: {
72-
+ StorageDead(_11);
73-
+ switchInt(_7) -> [1: bb2, otherwise: bb1];
89+
+ bb6: {
90+
+ StorageDead(_12);
91+
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb5];
7492
}
7593
}
7694

0 commit comments

Comments
 (0)