Skip to content

Commit

Permalink
Rollup merge of #66881 - krishna-veerareddy:issue-66780-bool-ord-opti…
Browse files Browse the repository at this point in the history
…mization, r=sfackler

Optimize Ord trait implementation for bool

Casting the booleans to `i8`s and converting their difference into `Ordering` generates better assembly than casting them to `u8`s and comparing them.

Fixes #66780

#### Comparison([Godbolt link](https://rust.godbolt.org/z/PjBpvF))

##### Old assembly:
```asm
example::boolean_cmp:
        mov     ecx, edi
        xor     ecx, esi
        test    esi, esi
        mov     eax, 255
        cmove   eax, ecx
        test    edi, edi
        cmovne  eax, ecx
        ret
```

##### New assembly:
```asm
example::boolean_cmp:
        mov     eax, edi
        sub     al, sil
        ret
```

##### Old LLVM-MCA statistics:
```
Iterations:        100
Instructions:      800
Total Cycles:      234
Total uOps:        1000

Dispatch Width:    6
uOps Per Cycle:    4.27
IPC:               3.42
Block RThroughput: 1.7
```

##### New LLVM-MCA statistics:
```
Iterations:        100
Instructions:      300
Total Cycles:      110
Total uOps:        500

Dispatch Width:    6
uOps Per Cycle:    4.55
IPC:               2.73
Block RThroughput: 1.0
```
  • Loading branch information
Centril authored Dec 11, 2019
2 parents ddca1e0 + 1f07aa5 commit 830b4ee
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/libcore/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,7 @@ pub fn max_by_key<T, F: FnMut(&T) -> K, K: Ord>(v1: T, v2: T, mut f: F) -> T {

// Implementation of PartialEq, Eq, PartialOrd and Ord for primitive types
mod impls {
use crate::hint::unreachable_unchecked;
use crate::cmp::Ordering::{self, Less, Greater, Equal};

macro_rules! partial_eq_impl {
Expand Down Expand Up @@ -1125,7 +1126,16 @@ mod impls {
impl Ord for bool {
#[inline]
fn cmp(&self, other: &bool) -> Ordering {
(*self as u8).cmp(&(*other as u8))
// Casting to i8's and converting the difference to an Ordering generates
// more optimal assembly.
// See <https://github.com/rust-lang/rust/issues/66780> for more info.
match (*self as i8) - (*other as i8) {
-1 => Less,
0 => Equal,
1 => Greater,
// SAFETY: bool as i8 returns 0 or 1, so the difference can't be anything else
_ => unsafe { unreachable_unchecked() },
}
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/libcore/tests/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ fn test_int_totalord() {
assert_eq!(12.cmp(&-5), Greater);
}

#[test]
fn test_bool_totalord() {
assert_eq!(true.cmp(&false), Greater);
assert_eq!(false.cmp(&true), Less);
assert_eq!(true.cmp(&true), Equal);
assert_eq!(false.cmp(&false), Equal);
}

#[test]
fn test_mut_int_totalord() {
assert_eq!((&mut 5).cmp(&&mut 10), Less);
Expand Down
17 changes: 17 additions & 0 deletions src/test/codegen/bool-cmp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// This is a test for optimal Ord trait implementation for bool.
// See <https://github.com/rust-lang/rust/issues/66780> for more info.

// compile-flags: -C opt-level=3

#![crate_type = "lib"]

use std::cmp::Ordering;

// CHECK-LABEL: @cmp_bool
#[no_mangle]
pub fn cmp_bool(a: bool, b: bool) -> Ordering {
// CHECK: zext i1
// CHECK: zext i1
// CHECK: sub nsw
a.cmp(&b)
}

0 comments on commit 830b4ee

Please sign in to comment.