Skip to content

Commit

Permalink
Auto merge of rust-lang#121397 - DianQK:early_otherwise_branch_sound,…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
bors committed Mar 3, 2024
2 parents 279c9ba + 73ef0c0 commit a4a739a
Show file tree
Hide file tree
Showing 20 changed files with 1,154 additions and 311 deletions.
451 changes: 243 additions & 208 deletions compiler/rustc_mir_transform/src/early_otherwise_branch.rs

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions tests/codegen/enum/enum-early-otherwise-branch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ compile-flags: -O

#![crate_type = "lib"]

pub enum Enum {
A(u32),
B(u32),
C(u32),
}

#[no_mangle]
pub fn foo(lhs: &Enum, rhs: &Enum) -> bool {
// CHECK-LABEL: define{{.*}}i1 @foo(
// CHECK-NOT: switch
// CHECK-NOT: br
// CHECK: [[SELECT:%.*]] = select
// CHECK-NEXT: ret i1 [[SELECT]]
// CHECK-NEXT: }
match (lhs, rhs) {
(Enum::A(lhs), Enum::A(rhs)) => lhs == rhs,
(Enum::B(lhs), Enum::B(rhs)) => lhs == rhs,
(Enum::C(lhs), Enum::C(rhs)) => lhs == rhs,
_ => false,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
let mut _7: isize;
let _8: u32;
let _9: u32;
+ let mut _10: isize;
+ let mut _11: bool;
scope 1 {
debug a => _8;
debug b => _9;
Expand All @@ -29,48 +27,33 @@
StorageDead(_5);
StorageDead(_4);
_7 = discriminant((_3.0: std::option::Option<u32>));
- switchInt(move _7) -> [1: bb2, otherwise: bb1];
+ StorageLive(_10);
+ _10 = discriminant((_3.1: std::option::Option<u32>));
+ StorageLive(_11);
+ _11 = Ne(_7, move _10);
+ StorageDead(_10);
+ switchInt(move _11) -> [0: bb4, otherwise: bb1];
switchInt(move _7) -> [1: bb2, otherwise: bb1];
}

bb1: {
+ StorageDead(_11);
_0 = const 1_u32;
- goto -> bb4;
+ goto -> bb3;
goto -> bb4;
}

bb2: {
- _6 = discriminant((_3.1: std::option::Option<u32>));
- switchInt(move _6) -> [1: bb3, otherwise: bb1];
- }
-
- bb3: {
_6 = discriminant((_3.1: std::option::Option<u32>));
switchInt(move _6) -> [1: bb3, otherwise: bb1];
}
bb3: {
StorageLive(_8);
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
StorageLive(_9);
_9 = (((_3.1: std::option::Option<u32>) as Some).0: u32);
_0 = const 0_u32;
StorageDead(_9);
StorageDead(_8);
- goto -> bb4;
+ goto -> bb3;
goto -> bb4;
}

- bb4: {
+ bb3: {
bb4: {
StorageDead(_3);
return;
+ }
+
+ bb4: {
+ StorageDead(_11);
+ switchInt(_7) -> [1: bb2, otherwise: bb1];
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
- // MIR for `opt10` before EarlyOtherwiseBranch
+ // MIR for `opt10` after EarlyOtherwiseBranch

fn opt10(_1: E8, _2: E16) -> u32 {
debug x => _1;
debug y => _2;
let mut _0: u32;
let mut _3: (E8, E16);
let mut _4: E8;
let mut _5: E16;
let mut _6: u16;
let mut _7: u16;
let mut _8: u16;
let mut _9: u8;
+ let mut _10: u16;

bb0: {
StorageLive(_3);
StorageLive(_4);
_4 = move _1;
StorageLive(_5);
_5 = move _2;
_3 = (move _4, move _5);
StorageDead(_5);
StorageDead(_4);
_9 = discriminant((_3.0: E8));
- switchInt(move _9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb9];
+ StorageLive(_10);
+ _10 = discriminant((_3.1: E16));
+ switchInt(move _10) -> [0: bb7, otherwise: bb1];
}

bb1: {
+ StorageDead(_10);
_0 = const 0_u32;
- goto -> bb8;
+ goto -> bb5;
}

bb2: {
- _6 = discriminant((_3.1: E16));
- switchInt(move _6) -> [0: bb5, otherwise: bb1];
+ _0 = const 1_u32;
+ goto -> bb5;
}

bb3: {
- _7 = discriminant((_3.1: E16));
- switchInt(move _7) -> [0: bb6, otherwise: bb1];
+ _0 = const 2_u32;
+ goto -> bb5;
}

bb4: {
- _8 = discriminant((_3.1: E16));
- switchInt(move _8) -> [0: bb7, otherwise: bb1];
+ _0 = const 3_u32;
+ goto -> bb5;
}

bb5: {
- _0 = const 1_u32;
- goto -> bb8;
+ StorageDead(_3);
+ return;
}

bb6: {
- _0 = const 2_u32;
- goto -> bb8;
+ unreachable;
}

bb7: {
- _0 = const 3_u32;
- goto -> bb8;
- }
-
- bb8: {
- StorageDead(_3);
- return;
- }
-
- bb9: {
- unreachable;
+ StorageDead(_10);
+ switchInt(_9) -> [0: bb2, 1: bb3, 2: bb4, otherwise: bb6];
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@
StorageDead(_5);
StorageDead(_4);
_8 = discriminant((_3.0: std::option::Option<u32>));
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb1];
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb7];
+ StorageLive(_11);
+ _11 = discriminant((_3.1: std::option::Option<u32>));
+ StorageLive(_12);
+ _12 = Ne(_8, move _11);
+ StorageDead(_11);
+ switchInt(move _12) -> [0: bb5, otherwise: bb1];
+ switchInt(move _12) -> [0: bb6, otherwise: bb1];
}

bb1: {
Expand Down Expand Up @@ -70,7 +70,7 @@

- bb5: {
+ bb3: {
_0 = const 0_u32;
_0 = const 2_u32;
- goto -> bb6;
+ goto -> bb4;
}
Expand All @@ -79,11 +79,16 @@
+ bb4: {
StorageDead(_3);
return;
}

- bb7: {
+ bb5: {
unreachable;
+ }
+
+ bb5: {
+ bb6: {
+ StorageDead(_12);
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb1];
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb5];
}
}

72 changes: 45 additions & 27 deletions tests/mir-opt/early_otherwise_branch.opt3.EarlyOtherwiseBranch.diff
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@
let mut _5: std::option::Option<bool>;
let mut _6: isize;
let mut _7: isize;
let _8: u32;
let _9: bool;
+ let mut _10: isize;
+ let mut _11: bool;
let mut _8: isize;
let _9: u32;
let _10: bool;
+ let mut _11: isize;
+ let mut _12: bool;
scope 1 {
debug a => _8;
debug b => _9;
debug a => _9;
debug b => _10;
}

bb0: {
Expand All @@ -28,49 +29,66 @@
_3 = (move _4, move _5);
StorageDead(_5);
StorageDead(_4);
_7 = discriminant((_3.0: std::option::Option<u32>));
- switchInt(move _7) -> [1: bb2, otherwise: bb1];
+ StorageLive(_10);
+ _10 = discriminant((_3.1: std::option::Option<bool>));
_8 = discriminant((_3.0: std::option::Option<u32>));
- switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb7];
+ StorageLive(_11);
+ _11 = Ne(_7, move _10);
+ StorageDead(_10);
+ switchInt(move _11) -> [0: bb4, otherwise: bb1];
+ _11 = discriminant((_3.1: std::option::Option<bool>));
+ StorageLive(_12);
+ _12 = Ne(_8, move _11);
+ StorageDead(_11);
+ switchInt(move _12) -> [0: bb6, otherwise: bb1];
}

bb1: {
+ StorageDead(_11);
+ StorageDead(_12);
_0 = const 1_u32;
- goto -> bb4;
+ goto -> bb3;
- goto -> bb6;
+ goto -> bb4;
}

bb2: {
- _6 = discriminant((_3.1: std::option::Option<bool>));
- switchInt(move _6) -> [1: bb3, otherwise: bb1];
- switchInt(move _6) -> [0: bb5, otherwise: bb1];
- }
-
- bb3: {
StorageLive(_8);
_8 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
- _7 = discriminant((_3.1: std::option::Option<bool>));
- switchInt(move _7) -> [1: bb4, otherwise: bb1];
- }
-
- bb4: {
StorageLive(_9);
_9 = (((_3.1: std::option::Option<bool>) as Some).0: bool);
_9 = (((_3.0: std::option::Option<u32>) as Some).0: u32);
StorageLive(_10);
_10 = (((_3.1: std::option::Option<bool>) as Some).0: bool);
_0 = const 0_u32;
StorageDead(_10);
StorageDead(_9);
StorageDead(_8);
- goto -> bb4;
+ goto -> bb3;
- goto -> bb6;
+ goto -> bb4;
}

- bb4: {
- bb5: {
+ bb3: {
_0 = const 2_u32;
- goto -> bb6;
+ goto -> bb4;
}

- bb6: {
+ bb4: {
StorageDead(_3);
return;
}

- bb7: {
+ bb5: {
unreachable;
+ }
+
+ bb4: {
+ StorageDead(_11);
+ switchInt(_7) -> [1: bb2, otherwise: bb1];
+ bb6: {
+ StorageDead(_12);
+ switchInt(_8) -> [0: bb3, 1: bb2, otherwise: bb5];
}
}

Loading

0 comments on commit a4a739a

Please sign in to comment.