replace if with match in binary_search

[a1393323447]

Now, the match version generates pretty much the same assembly as the if version. And I think match is more readable than if. Here is the code: https://rust.godbolt.org/z/4vPc54hb7

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cuviper (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[cuviper]

Now, the match version generates pretty much the same assembly as the if version.

It looks like they lined up starting with Rust 1.65, which is when we upgraded to LLVM 15.

However, in the few benchmarks we have, there's a measurable regression here.

Here's the merge-base commit 4781233, ./x bench core --test-args binary_search:

running 9 tests
test slice::binary_search_l1                                       ... bench:          29 ns/iter (+/- 1)
test slice::binary_search_l1_with_dups                             ... bench:          17 ns/iter (+/- 0)
test slice::binary_search_l1_worst_case                            ... bench:           4 ns/iter (+/- 0)
test slice::binary_search_l2                                       ... bench:          42 ns/iter (+/- 0)
test slice::binary_search_l2_with_dups                             ... bench:          28 ns/iter (+/- 0)
test slice::binary_search_l2_worst_case                            ... bench:           6 ns/iter (+/- 0)
test slice::binary_search_l3                                       ... bench:         102 ns/iter (+/- 2)
test slice::binary_search_l3_with_dups                             ... bench:          84 ns/iter (+/- 1)
test slice::binary_search_l3_worst_case                            ... bench:          11 ns/iter (+/- 1)

With your change:

running 9 tests
test slice::binary_search_l1                                       ... bench:          33 ns/iter (+/- 0)
test slice::binary_search_l1_with_dups                             ... bench:          19 ns/iter (+/- 0)
test slice::binary_search_l1_worst_case                            ... bench:           7 ns/iter (+/- 0)
test slice::binary_search_l2                                       ... bench:          49 ns/iter (+/- 0)
test slice::binary_search_l2_with_dups                             ... bench:          31 ns/iter (+/- 0)
test slice::binary_search_l2_worst_case                            ... bench:          11 ns/iter (+/- 0)
test slice::binary_search_l3                                       ... bench:         111 ns/iter (+/- 1)
test slice::binary_search_l3_with_dups                             ... bench:          91 ns/iter (+/- 1)
test slice::binary_search_l3_worst_case                            ... bench:          17 ns/iter (+/- 1)

The benchmarks are pretty small, but these results are very consistent across multiple runs.

(I'm running x86_64-unknown-linux-gnu on an AMD Ryzen 7 5800X and Fedora 37.)

cc @Folyd as the author of #74024.

[scottmcm]

FWIW, getting Ordering checks to optimize well is seemingly a huge rabbit hole.

I've got two related LLVM bugs open (llvm/llvm-project#59666 & llvm/llvm-project#60012), #105840 is trying to make cmp better but hitting some weird optimization issues, #106107 shows that they often don't work great, and I tried #106065 but it didn't quite work either, leading me to open yet another LLVM bug (llvm/llvm-project#59668).

Thus without a strong argument that the match way is better, I'd be inclined to just leave it as-is. (Things like binary search in core are the kind of places where we'll accept slightly less canonical code in exchange for perf.)