Skip to content

Commit

Permalink
Auto merge of #122459 - Nadrieril:sort-eq, r=oli-obk
Browse files Browse the repository at this point in the history
match lowering: sort `Eq` candidates in the failure case too

This is a slight tweak to MIR gen of matches. Take a match like:
```rust
match (s, flag) {
    ("a", _) if foo() => 1,
    ("b", true) => 2,
    ("a", false) => 3,
    (_, true) => 4,
    _ => 5,
}
```
If we switch on `s == "a"`, the first candidate matches, and we learn almost nothing about the second candidate. So there's a choice:
1. (what we do today) stop sorting candidates, keep the "b" case grouped with everything below. This could allow us to be clever here and test on `flag == true` next.
2. (what this PR does) sort "b" into the failure case. The "b" will be alone (fewer opportunities for picking a good test), but that means the two "a" cases require a single test.

Today, we aren't clever in which tests we pick, so this is an unambiguous win. In a future where we pick tests better, idk. Grouping tests as much as possible feels like a generally good strategy.

This was proposed in #29623 (9 years ago :D)
  • Loading branch information
bors committed Mar 31, 2024
2 parents 1aedc96 + 65efa5b commit 5baf1e1
Show file tree
Hide file tree
Showing 16 changed files with 255 additions and 131 deletions.
14 changes: 8 additions & 6 deletions compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,12 +650,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

// FIXME(#29623): return `Some(1)` when the values are different.
(TestKind::Eq { value: test_val, .. }, TestCase::Constant { value: case_val })
if test_val == case_val =>
{
fully_matched = true;
Some(TestBranch::Success)
(TestKind::Eq { value: test_val, .. }, TestCase::Constant { value: case_val }) => {
if test_val == case_val {
fully_matched = true;
Some(TestBranch::Success)
} else {
fully_matched = false;
Some(TestBranch::Failure)
}
}

(
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// MIR for `constant_eq` after SimplifyCfg-initial

fn constant_eq(_1: &str, _2: bool) -> u32 {
debug s => _1;
debug b => _2;
let mut _0: u32;
let mut _3: (&str, bool);
let mut _4: &str;
let mut _5: bool;
let mut _6: bool;
let mut _7: bool;
let mut _8: &&str;
let mut _9: &bool;
let mut _10: bool;

bb0: {
StorageLive(_3);
StorageLive(_4);
_4 = _1;
StorageLive(_5);
_5 = _2;
_3 = (move _4, move _5);
StorageDead(_5);
StorageDead(_4);
PlaceMention(_3);
_7 = <str as PartialEq>::eq((_3.0: &str), const "a") -> [return: bb11, unwind: bb19];
}

bb1: {
switchInt((_3.1: bool)) -> [0: bb2, otherwise: bb3];
}

bb2: {
_0 = const 5_u32;
goto -> bb18;
}

bb3: {
falseEdge -> [real: bb17, imaginary: bb2];
}

bb4: {
falseEdge -> [real: bb12, imaginary: bb9];
}

bb5: {
switchInt((_3.1: bool)) -> [0: bb1, otherwise: bb6];
}

bb6: {
falseEdge -> [real: bb16, imaginary: bb3];
}

bb7: {
_6 = <str as PartialEq>::eq((_3.0: &str), const "b") -> [return: bb10, unwind: bb19];
}

bb8: {
switchInt((_3.1: bool)) -> [0: bb1, otherwise: bb9];
}

bb9: {
falseEdge -> [real: bb15, imaginary: bb6];
}

bb10: {
switchInt(move _6) -> [0: bb1, otherwise: bb8];
}

bb11: {
switchInt(move _7) -> [0: bb7, otherwise: bb4];
}

bb12: {
_8 = &fake (_3.0: &str);
_9 = &fake (_3.1: bool);
StorageLive(_10);
_10 = const true;
switchInt(move _10) -> [0: bb14, otherwise: bb13];
}

bb13: {
StorageDead(_10);
FakeRead(ForMatchGuard, _8);
FakeRead(ForMatchGuard, _9);
_0 = const 1_u32;
goto -> bb18;
}

bb14: {
StorageDead(_10);
falseEdge -> [real: bb5, imaginary: bb9];
}

bb15: {
_0 = const 2_u32;
goto -> bb18;
}

bb16: {
_0 = const 3_u32;
goto -> bb18;
}

bb17: {
_0 = const 4_u32;
goto -> bb18;
}

bb18: {
StorageDead(_3);
return;
}

bb19 (cleanup): {
resume;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// MIR for `disjoint_ranges` after SimplifyCfg-initial

fn disjoint_ranges(_1: i32, _2: bool) -> u32 {
debug x => _1;
debug b => _2;
let mut _0: u32;
let mut _3: bool;
let mut _4: bool;
let mut _5: bool;
let mut _6: bool;
let mut _7: &i32;
let mut _8: bool;

bb0: {
PlaceMention(_1);
_5 = Le(const 0_i32, _1);
switchInt(move _5) -> [0: bb3, otherwise: bb8];
}

bb1: {
_0 = const 3_u32;
goto -> bb14;
}

bb2: {
falseEdge -> [real: bb9, imaginary: bb4];
}

bb3: {
_3 = Le(const 10_i32, _1);
switchInt(move _3) -> [0: bb5, otherwise: bb7];
}

bb4: {
falseEdge -> [real: bb12, imaginary: bb6];
}

bb5: {
switchInt(_1) -> [4294967295: bb6, otherwise: bb1];
}

bb6: {
falseEdge -> [real: bb13, imaginary: bb1];
}

bb7: {
_4 = Le(_1, const 20_i32);
switchInt(move _4) -> [0: bb5, otherwise: bb4];
}

bb8: {
_6 = Lt(_1, const 10_i32);
switchInt(move _6) -> [0: bb3, otherwise: bb2];
}

bb9: {
_7 = &fake _1;
StorageLive(_8);
_8 = _2;
switchInt(move _8) -> [0: bb11, otherwise: bb10];
}

bb10: {
StorageDead(_8);
FakeRead(ForMatchGuard, _7);
_0 = const 0_u32;
goto -> bb14;
}

bb11: {
StorageDead(_8);
falseEdge -> [real: bb1, imaginary: bb4];
}

bb12: {
_0 = const 1_u32;
goto -> bb14;
}

bb13: {
_0 = const 2_u32;
goto -> bb14;
}

bb14: {
return;
}
}
41 changes: 41 additions & 0 deletions tests/mir-opt/building/match/sort_candidates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Check specific cases of sorting candidates in match lowering.
#![feature(exclusive_range_pattern)]

// EMIT_MIR sort_candidates.constant_eq.SimplifyCfg-initial.after.mir
fn constant_eq(s: &str, b: bool) -> u32 {
// Check that we only test "a" once

// CHECK-LABEL: fn constant_eq(
// CHECK: bb0: {
// CHECK: [[a:_.*]] = const "a";
// CHECK-NOT: {{_.*}} = const "a";
match (s, b) {
("a", _) if true => 1,
("b", true) => 2,
("a", true) => 3,
(_, true) => 4,
_ => 5,
}
}

// EMIT_MIR sort_candidates.disjoint_ranges.SimplifyCfg-initial.after.mir
fn disjoint_ranges(x: i32, b: bool) -> u32 {
// When `(0..=10).contains(x) && !b`, we should jump to the last arm without testing the two
// other candidates.

// CHECK-LABEL: fn disjoint_ranges(
// CHECK: debug b => _2;
// CHECK: bb0: {
// CHECK: switchInt(_2) -> [0: [[jump:bb.*]], otherwise: {{bb.*}}];
// CHECK: [[jump]]: {
// CHECK-NEXT: _0 = const 3_u32;
// CHECK-NEXT: return;
match x {
0..10 if b => 0,
10..=20 => 1,
-1 => 2,
_ => 3,
}
}

fn main() {}
106 changes: 0 additions & 106 deletions tests/mir-opt/match_test.main.SimplifyCfg-initial.after.mir

This file was deleted.

19 changes: 0 additions & 19 deletions tests/mir-opt/match_test.rs

This file was deleted.

0 comments on commit 5baf1e1

Please sign in to comment.