Micro-optimize Ord::cmp for primitives

Author: saethlin

compiler/rustc_span/src/lib.rs

@@ -2191,6 +2191,50 @@ macro_rules! impl_pos {
             $(#[$attr])*
             $vis struct $ident($inner_vis $inner_ty);
 
+            impl ::std::cmp::Ord for $ident {
+                #[inline(always)]
+                fn cmp(&self, other: &Self) -> ::std::cmp::Ordering {
+                    self.0.cmp(&other.0)
+                }
+
+                #[inline(always)]
+                fn min(self, other: Self) -> Self {
+                    Self(self.0.min(other.0))
+                }
+
+                #[inline(always)]
+                fn max(self, other: Self) -> Self {
+                    Self(self.0.max(other.0))
+                }
+            }
+
+            impl ::std::cmp::PartialOrd for $ident {
+                #[inline(always)]
+                fn partial_cmp(&self, other: &Self) -> Option<::std::cmp::Ordering> {
+                    self.0.partial_cmp(&other.0)
+                }
+
+                #[inline(always)]
+                fn lt(&self, other: &Self) -> bool {
+                    self.0.lt(&other.0)
+                }
+
+                #[inline(always)]
+                fn le(&self, other: &Self) -> bool {
+                    self.0.le(&other.0)
+                }
+
+                #[inline(always)]
+                fn gt(&self, other: &Self) -> bool {
+                    self.0.gt(&other.0)
+                }
+
+                #[inline(always)]
+                fn ge(&self, other: &Self) -> bool {
+                    self.0.ge(&other.0)
+                }
+            }
+
             impl Pos for $ident {
                 #[inline(always)]
                 fn from_usize(n: usize) -> $ident {
@@ -2238,7 +2282,7 @@ impl_pos! {
     /// A byte offset.
     ///
     /// Keep this small (currently 32-bits), as AST contains a lot of them.
-    #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Debug)]
+    #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
     pub struct BytePos(pub u32);
 
     /// A byte offset relative to file beginning.
@@ -2250,7 +2294,7 @@ impl_pos! {
     /// Because of multibyte UTF-8 characters, a byte offset
     /// is not equivalent to a character offset. The [`SourceMap`] will convert [`BytePos`]
     /// values to `CharPos` values as necessary.
-    #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
+    #[derive(Clone, Copy, PartialEq, Eq, Debug)]
     pub struct CharPos(pub usize);
 }
 

library/core/src/cmp.rs

@@ -401,12 +401,18 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_eq(), true);
     /// assert_eq!(Ordering::Greater.is_eq(), false);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_eq(self) -> bool {
-        matches!(self, Equal)
+        // Implementation note: It appears (as of 2022-12) that LLVM has an
+        // easier time with a comparison against zero like this, as opposed
+        // to looking for the `Less` value (-1) specifically, maybe because
+        // it's not always obvious to it that -2 isn't possible.
+        // Thus this and its siblings below are written this way, rather
+        // than the potentially-more-obvious `matches!` version.
+        (self as i8) == 0
     }
 
     /// Returns `true` if the ordering is not the `Equal` variant.
@@ -420,12 +426,12 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_ne(), false);
     /// assert_eq!(Ordering::Greater.is_ne(), true);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_ne(self) -> bool {
-        !matches!(self, Equal)
+        (self as i8) != 0
     }
 
     /// Returns `true` if the ordering is the `Less` variant.
@@ -439,12 +445,12 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_lt(), false);
     /// assert_eq!(Ordering::Greater.is_lt(), false);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_lt(self) -> bool {
-        matches!(self, Less)
+        (self as i8) < 0
     }
 
     /// Returns `true` if the ordering is the `Greater` variant.
@@ -458,12 +464,12 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_gt(), false);
     /// assert_eq!(Ordering::Greater.is_gt(), true);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_gt(self) -> bool {
-        matches!(self, Greater)
+        (self as i8) > 0
     }
 
     /// Returns `true` if the ordering is either the `Less` or `Equal` variant.
@@ -477,12 +483,12 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_le(), true);
     /// assert_eq!(Ordering::Greater.is_le(), false);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_le(self) -> bool {
-        !matches!(self, Greater)
+        (self as i8) <= 0
     }
 
     /// Returns `true` if the ordering is either the `Greater` or `Equal` variant.
@@ -496,12 +502,12 @@ impl Ordering {
     /// assert_eq!(Ordering::Equal.is_ge(), true);
     /// assert_eq!(Ordering::Greater.is_ge(), true);
     /// ```
-    #[inline]
+    #[inline(always)]
     #[must_use]
     #[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
     #[stable(feature = "ordering_helpers", since = "1.53.0")]
     pub const fn is_ge(self) -> bool {
-        !matches!(self, Less)
+        (self as i8) >= 0
     }
 
     /// Reverses the `Ordering`.
@@ -1169,7 +1175,11 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn lt(&self, other: &Rhs) -> bool {
-        matches!(self.partial_cmp(other), Some(Less))
+        if let Some(ordering) = self.partial_cmp(other) {
+            ordering.is_lt()
+        } else {
+            false
+        }
     }
 
     /// This method tests less than or equal to (for `self` and `other`) and is used by the `<=`
@@ -1186,7 +1196,11 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn le(&self, other: &Rhs) -> bool {
-        matches!(self.partial_cmp(other), Some(Less | Equal))
+        if let Some(ordering) = self.partial_cmp(other) {
+            ordering.is_le()
+        } else {
+            false
+        }
     }
 
     /// This method tests greater than (for `self` and `other`) and is used by the `>` operator.
@@ -1202,7 +1216,11 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn gt(&self, other: &Rhs) -> bool {
-        matches!(self.partial_cmp(other), Some(Greater))
+        if let Some(ordering) = self.partial_cmp(other) {
+            ordering.is_gt()
+        } else {
+            false
+        }
     }
 
     /// This method tests greater than or equal to (for `self` and `other`) and is used by the `>=`
@@ -1219,7 +1237,11 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
     #[must_use]
     #[stable(feature = "rust1", since = "1.0.0")]
     fn ge(&self, other: &Rhs) -> bool {
-        matches!(self.partial_cmp(other), Some(Greater | Equal))
+        if let Some(ordering) = self.partial_cmp(other) {
+            ordering.is_ge()
+        } else {
+            false
+        }
     }
 }
 
@@ -1563,12 +1585,31 @@ mod impls {
             impl Ord for $t {
                 #[inline]
                 fn cmp(&self, other: &$t) -> Ordering {
-                    // The order here is important to generate more optimal assembly.
-                    // See <https://github.com/rust-lang/rust/issues/63758> for more info.
-                    if *self < *other { Less }
-                    else if *self == *other { Equal }
-                    else { Greater }
+                    let mut res = 0i8;
+                    res -= (*self < *other) as i8;
+                    res += (*self > *other) as i8;
+                    // SAFETY: The discriminants of Ord were chosen to permit this
+                    unsafe { crate::mem::transmute(res) }
                 }
+
+                #[inline]
+                fn max(self, other: Self) -> Self {
+                    if self > other {
+                        self
+                    } else {
+                        other
+                    }
+                }
+
+                #[inline]
+                fn min(self, other: Self) -> Self {
+                    if self > other {
+                        other
+                    } else {
+                        self
+                    }
+                }
+
             }
         )*)
     }

tests/codegen/newtype-relational-operators.rs

@@ -0,0 +1,49 @@
+// The `derive(PartialOrd)` for a newtype doesn't override `lt`/`le`/`gt`/`ge`.
+// This double-checks that the `Option<Ordering>` intermediate values used
+// in the operators for such a type all optimize away.
+
+// compile-flags: -C opt-level=1
+// min-llvm-version: 15.0
+
+#![crate_type = "lib"]
+
+use std::cmp::Ordering;
+
+#[derive(PartialOrd, PartialEq)]
+pub struct Foo(u16);
+
+// CHECK-LABEL: @check_lt
+// CHECK-SAME: (i16 %[[A:.+]], i16 %[[B:.+]])
+#[no_mangle]
+pub fn check_lt(a: Foo, b: Foo) -> bool {
+    // CHECK: %[[R:.+]] = icmp ult i16 %[[A]], %[[B]]
+    // CHECK-NEXT: ret i1 %[[R]]
+    a < b
+}
+
+// CHECK-LABEL: @check_le
+// CHECK-SAME: (i16 %[[A:.+]], i16 %[[B:.+]])
+#[no_mangle]
+pub fn check_le(a: Foo, b: Foo) -> bool {
+    // CHECK: %[[R:.+]] = icmp ule i16 %[[A]], %[[B]]
+    // CHECK-NEXT: ret i1 %[[R]]
+    a <= b
+}
+
+// CHECK-LABEL: @check_gt
+// CHECK-SAME: (i16 %[[A:.+]], i16 %[[B:.+]])
+#[no_mangle]
+pub fn check_gt(a: Foo, b: Foo) -> bool {
+    // CHECK: %[[R:.+]] = icmp ugt i16 %[[A]], %[[B]]
+    // CHECK-NEXT: ret i1 %[[R]]
+    a > b
+}
+
+// CHECK-LABEL: @check_ge
+// CHECK-SAME: (i16 %[[A:.+]], i16 %[[B:.+]])
+#[no_mangle]
+pub fn check_ge(a: Foo, b: Foo) -> bool {
+    // CHECK: %[[R:.+]] = icmp uge i16 %[[A]], %[[B]]
+    // CHECK-NEXT: ret i1 %[[R]]
+    a >= b
+}