From 49de875af906391244b4e7975ed4904910647da5 Mon Sep 17 00:00:00 2001 From: okaneco <47607823+okaneco@users.noreply.github.com> Date: Wed, 8 Nov 2023 14:44:13 -0500 Subject: [PATCH] Refactor `binary_search_by` to use conditional moves Refactor the if/else checking on cmp::Ordering variants to a "branchless" reassignment of left and right. This change results in fewer branches and instructions. --- library/core/src/slice/mod.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 45080eda2ce2f..9075a3bb8e86a 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -6,7 +6,7 @@ #![stable(feature = "rust1", since = "1.0.0")] -use crate::cmp::Ordering::{self, Greater, Less}; +use crate::cmp::Ordering::{self, Equal, Greater, Less}; use crate::fmt; use crate::intrinsics::{assert_unsafe_precondition, exact_div}; use crate::marker::Copy; @@ -2844,14 +2844,13 @@ impl [T] { // we have `left + size/2 < self.len()`, and this is in-bounds. let cmp = f(unsafe { self.get_unchecked(mid) }); - // The reason why we use if/else control flow rather than match - // is because match reorders comparison operations, which is perf sensitive. - // This is x86 asm for u8: https://rust.godbolt.org/z/8Y8Pra. - if cmp == Less { - left = mid + 1; - } else if cmp == Greater { - right = mid; - } else { + // This control flow produces conditional moves, which results in + // fewer branches and instructions than if/else or matching on + // cmp::Ordering. + // This is x86 asm for u8: https://rust.godbolt.org/z/698eYffTx. + left = if cmp == Less { mid + 1 } else { left }; + right = if cmp == Greater { mid } else { right }; + if cmp == Equal { // SAFETY: same as the `get_unchecked` above unsafe { crate::intrinsics::assume(mid < self.len()) }; return Ok(mid);