Skip to content

Commit

Permalink
Rollup merge of rust-lang#137904 - scottmcm:ordering-is, r=workingjub…
Browse files Browse the repository at this point in the history
…ilee

Improve the generic MIR in the default `PartialOrd::le` and friends

It looks like I regressed this accidentally in rust-lang#137197 due to rust-lang#137901

So this PR does two things:
1. Tweaks the way we're calling `is_some_and` so that it optimizes in the generic MIR (rather than needing to optimize it in every monomorphization) -- the first commit adds a MIR test, so you can see the difference in the second commit.
2. Updates the implementations of `is_le` and friends to be slightly simpler, and parallel how clang does them.
  • Loading branch information
matthiaskrgr authored Mar 7, 2025
2 parents a98df54 + eae5ed6 commit 9e16082
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 9 deletions.
22 changes: 16 additions & 6 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,12 @@ pub enum Ordering {
}

impl Ordering {
#[inline]
const fn as_raw(self) -> i8 {
// FIXME(const-hack): just use `PartialOrd` against `Equal` once that's const
crate::intrinsics::discriminant_value(&self)
}

/// Returns `true` if the ordering is the `Equal` variant.
///
/// # Examples
Expand All @@ -413,7 +419,11 @@ impl Ordering {
#[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)
// All the `is_*` methods are implemented as comparisons against zero
// to follow how clang's libcxx implements their equivalents in
// <https://github.com/llvm/llvm-project/blob/60486292b79885b7800b082754153202bef5b1f0/libcxx/include/__compare/is_eq.h#L23-L28>

self.as_raw() == 0
}

/// Returns `true` if the ordering is not the `Equal` variant.
Expand All @@ -432,7 +442,7 @@ impl Ordering {
#[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_raw() != 0
}

/// Returns `true` if the ordering is the `Less` variant.
Expand All @@ -451,7 +461,7 @@ impl Ordering {
#[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_raw() < 0
}

/// Returns `true` if the ordering is the `Greater` variant.
Expand All @@ -470,7 +480,7 @@ impl Ordering {
#[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_raw() > 0
}

/// Returns `true` if the ordering is either the `Less` or `Equal` variant.
Expand All @@ -489,7 +499,7 @@ impl Ordering {
#[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_raw() <= 0
}

/// Returns `true` if the ordering is either the `Greater` or `Equal` variant.
Expand All @@ -508,7 +518,7 @@ impl Ordering {
#[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_raw() >= 0
}

/// Reverses the `Ordering`.
Expand Down
89 changes: 89 additions & 0 deletions tests/mir-opt/pre-codegen/derived_ord.demo_le.PreCodegen.after.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// MIR for `demo_le` after PreCodegen

fn demo_le(_1: &MultiField, _2: &MultiField) -> bool {
debug a => _1;
debug b => _2;
let mut _0: bool;
scope 1 (inlined <MultiField as PartialOrd>::le) {
let mut _11: std::option::Option<std::cmp::Ordering>;
scope 2 (inlined Option::<std::cmp::Ordering>::is_some_and::<fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le}>) {
let _12: std::cmp::Ordering;
scope 3 {
scope 4 (inlined <fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le} as FnOnce<(std::cmp::Ordering,)>>::call_once - shim(fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le})) {
scope 5 (inlined std::cmp::Ordering::is_le) {
let mut _13: i8;
scope 6 (inlined std::cmp::Ordering::as_raw) {
}
}
}
}
}
scope 7 (inlined <MultiField as PartialOrd>::partial_cmp) {
let mut _6: std::option::Option<std::cmp::Ordering>;
let mut _7: i8;
scope 8 {
}
scope 9 (inlined std::cmp::impls::<impl PartialOrd for char>::partial_cmp) {
let mut _3: char;
let mut _4: char;
let mut _5: std::cmp::Ordering;
}
scope 10 (inlined std::cmp::impls::<impl PartialOrd for i16>::partial_cmp) {
let mut _8: i16;
let mut _9: i16;
let mut _10: std::cmp::Ordering;
}
}
}

bb0: {
StorageLive(_12);
StorageLive(_11);
StorageLive(_5);
StorageLive(_7);
StorageLive(_3);
_3 = copy ((*_1).0: char);
StorageLive(_4);
_4 = copy ((*_2).0: char);
_5 = Cmp(move _3, move _4);
StorageDead(_4);
StorageDead(_3);
_6 = Option::<std::cmp::Ordering>::Some(copy _5);
_7 = discriminant(_5);
switchInt(move _7) -> [0: bb1, otherwise: bb2];
}

bb1: {
StorageLive(_10);
StorageLive(_8);
_8 = copy ((*_1).1: i16);
StorageLive(_9);
_9 = copy ((*_2).1: i16);
_10 = Cmp(move _8, move _9);
StorageDead(_9);
StorageDead(_8);
_11 = Option::<std::cmp::Ordering>::Some(move _10);
StorageDead(_10);
StorageDead(_7);
StorageDead(_5);
goto -> bb3;
}

bb2: {
_11 = copy _6;
StorageDead(_7);
StorageDead(_5);
goto -> bb3;
}

bb3: {
_12 = move ((_11 as Some).0: std::cmp::Ordering);
StorageLive(_13);
_13 = discriminant(_12);
_0 = Le(move _13, const 0_i8);
StorageDead(_13);
StorageDead(_11);
StorageDead(_12);
return;
}
}
26 changes: 25 additions & 1 deletion tests/mir-opt/pre-codegen/derived_ord.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,33 @@
// skip-filecheck
//@ compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=0

#![crate_type = "lib"]

#[derive(PartialOrd, PartialEq)]
pub struct MultiField(char, i16);

// Because this isn't derived by the impl, it's not on the `{impl#0}-partial_cmp`,
// and thus we need to call it to see what the inlined generic one produces.
pub fn demo_le(a: &MultiField, b: &MultiField) -> bool {
// CHECK-LABEL: fn demo_le
// CHECK: inlined <MultiField as PartialOrd>::le
// CHECK: inlined{{.+}}is_some_and
// CHECK: inlined <MultiField as PartialOrd>::partial_cmp

// CHECK: [[A0:_[0-9]+]] = copy ((*_1).0: char);
// CHECK: [[B0:_[0-9]+]] = copy ((*_2).0: char);
// CHECK: Cmp(move [[A0]], move [[B0]]);

// CHECK: [[D0:_[0-9]+]] = discriminant({{.+}});
// CHECK: switchInt(move [[D0]]) -> [0: bb{{[0-9]+}}, otherwise: bb{{[0-9]+}}];

// CHECK: [[A1:_[0-9]+]] = copy ((*_1).1: i16);
// CHECK: [[B1:_[0-9]+]] = copy ((*_2).1: i16);
// CHECK: Cmp(move [[A1]], move [[B1]]);

// CHECK: [[D1:_[0-9]+]] = discriminant({{.+}});
// CHECK: _0 = Le(move [[D1]], const 0_i8);
*a <= *b
}

// EMIT_MIR derived_ord.{impl#0}-partial_cmp.PreCodegen.after.mir
// EMIT_MIR derived_ord.demo_le.PreCodegen.after.mir
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// MIR for `<impl at $DIR/derived_ord.rs:6:10: 6:20>::partial_cmp` after PreCodegen
// MIR for `<impl at $DIR/derived_ord.rs:5:10: 5:20>::partial_cmp` after PreCodegen

fn <impl at $DIR/derived_ord.rs:6:10: 6:20>::partial_cmp(_1: &MultiField, _2: &MultiField) -> Option<std::cmp::Ordering> {
fn <impl at $DIR/derived_ord.rs:5:10: 5:20>::partial_cmp(_1: &MultiField, _2: &MultiField) -> Option<std::cmp::Ordering> {
debug self => _1;
debug other => _2;
let mut _0: std::option::Option<std::cmp::Ordering>;
Expand Down

0 comments on commit 9e16082

Please sign in to comment.