Skip to content

Commit 5c944f0

Browse files
committed
Auto merge of #137904 - scottmcm:ordering-is, r=<try>
Improve the generic MIR in the default `PartialOrd::le` and friends It looks like I regressed this accidentally in #137197 due to #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.
2 parents f4a216d + fcb91b8 commit 5c944f0

File tree

4 files changed

+138
-13
lines changed

4 files changed

+138
-13
lines changed

library/core/src/cmp.rs

+22-10
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,12 @@ pub enum Ordering {
397397
}
398398

399399
impl Ordering {
400+
#[inline]
401+
const fn as_raw(self) -> i8 {
402+
// FIXME(const-hack): just use `PartialOrd` against `Equal` once that's const
403+
crate::intrinsics::discriminant_value(&self)
404+
}
405+
400406
/// Returns `true` if the ordering is the `Equal` variant.
401407
///
402408
/// # Examples
@@ -413,7 +419,11 @@ impl Ordering {
413419
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
414420
#[stable(feature = "ordering_helpers", since = "1.53.0")]
415421
pub const fn is_eq(self) -> bool {
416-
matches!(self, Equal)
422+
// All the `is_*` methods are implemented as comparisons against zero
423+
// to follow how clang's libcxx implements their equivalents in
424+
// <https://github.com/llvm/llvm-project/blob/60486292b79885b7800b082754153202bef5b1f0/libcxx/include/__compare/is_eq.h#L23-L28>
425+
426+
self.as_raw() == 0
417427
}
418428

419429
/// Returns `true` if the ordering is not the `Equal` variant.
@@ -432,7 +442,7 @@ impl Ordering {
432442
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
433443
#[stable(feature = "ordering_helpers", since = "1.53.0")]
434444
pub const fn is_ne(self) -> bool {
435-
!matches!(self, Equal)
445+
self.as_raw() != 0
436446
}
437447

438448
/// Returns `true` if the ordering is the `Less` variant.
@@ -451,7 +461,7 @@ impl Ordering {
451461
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
452462
#[stable(feature = "ordering_helpers", since = "1.53.0")]
453463
pub const fn is_lt(self) -> bool {
454-
matches!(self, Less)
464+
self.as_raw() < 0
455465
}
456466

457467
/// Returns `true` if the ordering is the `Greater` variant.
@@ -470,7 +480,7 @@ impl Ordering {
470480
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
471481
#[stable(feature = "ordering_helpers", since = "1.53.0")]
472482
pub const fn is_gt(self) -> bool {
473-
matches!(self, Greater)
483+
self.as_raw() > 0
474484
}
475485

476486
/// Returns `true` if the ordering is either the `Less` or `Equal` variant.
@@ -489,7 +499,7 @@ impl Ordering {
489499
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
490500
#[stable(feature = "ordering_helpers", since = "1.53.0")]
491501
pub const fn is_le(self) -> bool {
492-
!matches!(self, Greater)
502+
self.as_raw() <= 0
493503
}
494504

495505
/// Returns `true` if the ordering is either the `Greater` or `Equal` variant.
@@ -508,7 +518,7 @@ impl Ordering {
508518
#[rustc_const_stable(feature = "ordering_helpers", since = "1.53.0")]
509519
#[stable(feature = "ordering_helpers", since = "1.53.0")]
510520
pub const fn is_ge(self) -> bool {
511-
!matches!(self, Less)
521+
self.as_raw() >= 0
512522
}
513523

514524
/// Reverses the `Ordering`.
@@ -1369,7 +1379,9 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
13691379
#[stable(feature = "rust1", since = "1.0.0")]
13701380
#[rustc_diagnostic_item = "cmp_partialord_lt"]
13711381
fn lt(&self, other: &Rhs) -> bool {
1372-
self.partial_cmp(other).is_some_and(Ordering::is_lt)
1382+
// FIXME(#137901): weirdly, passing these as `Ordering::is_lt` doesn't
1383+
// MIR-inline, thus the "unnecessary" closure form here.
1384+
self.partial_cmp(other).is_some_and(|c| c.is_lt())
13731385
}
13741386

13751387
/// Tests less than or equal to (for `self` and `other`) and is used by the
@@ -1387,7 +1399,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
13871399
#[stable(feature = "rust1", since = "1.0.0")]
13881400
#[rustc_diagnostic_item = "cmp_partialord_le"]
13891401
fn le(&self, other: &Rhs) -> bool {
1390-
self.partial_cmp(other).is_some_and(Ordering::is_le)
1402+
self.partial_cmp(other).is_some_and(|c| c.is_le())
13911403
}
13921404

13931405
/// Tests greater than (for `self` and `other`) and is used by the `>`
@@ -1405,7 +1417,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
14051417
#[stable(feature = "rust1", since = "1.0.0")]
14061418
#[rustc_diagnostic_item = "cmp_partialord_gt"]
14071419
fn gt(&self, other: &Rhs) -> bool {
1408-
self.partial_cmp(other).is_some_and(Ordering::is_gt)
1420+
self.partial_cmp(other).is_some_and(|c| c.is_gt())
14091421
}
14101422

14111423
/// Tests greater than or equal to (for `self` and `other`) and is used by
@@ -1423,7 +1435,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
14231435
#[stable(feature = "rust1", since = "1.0.0")]
14241436
#[rustc_diagnostic_item = "cmp_partialord_ge"]
14251437
fn ge(&self, other: &Rhs) -> bool {
1426-
self.partial_cmp(other).is_some_and(Ordering::is_ge)
1438+
self.partial_cmp(other).is_some_and(|c| c.is_ge())
14271439
}
14281440
}
14291441

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// MIR for `demo_le` after PreCodegen
2+
3+
fn demo_le(_1: &MultiField, _2: &MultiField) -> bool {
4+
debug a => _1;
5+
debug b => _2;
6+
let mut _0: bool;
7+
scope 1 (inlined <MultiField as PartialOrd>::le) {
8+
let mut _11: std::option::Option<std::cmp::Ordering>;
9+
scope 2 (inlined Option::<std::cmp::Ordering>::is_some_and::<{closure@<MultiField as PartialOrd>::le::{closure#0}}>) {
10+
let _12: std::cmp::Ordering;
11+
scope 3 {
12+
scope 4 (inlined <MultiField as PartialOrd>::le::{closure#0}) {
13+
scope 5 (inlined std::cmp::Ordering::is_le) {
14+
let mut _13: i8;
15+
scope 6 (inlined std::cmp::Ordering::as_raw) {
16+
}
17+
}
18+
}
19+
}
20+
}
21+
scope 7 (inlined <MultiField as PartialOrd>::partial_cmp) {
22+
let mut _6: std::option::Option<std::cmp::Ordering>;
23+
let mut _7: i8;
24+
scope 8 {
25+
}
26+
scope 9 (inlined std::cmp::impls::<impl PartialOrd for char>::partial_cmp) {
27+
let mut _3: char;
28+
let mut _4: char;
29+
let mut _5: std::cmp::Ordering;
30+
}
31+
scope 10 (inlined std::cmp::impls::<impl PartialOrd for i16>::partial_cmp) {
32+
let mut _8: i16;
33+
let mut _9: i16;
34+
let mut _10: std::cmp::Ordering;
35+
}
36+
}
37+
}
38+
39+
bb0: {
40+
StorageLive(_12);
41+
StorageLive(_11);
42+
StorageLive(_5);
43+
StorageLive(_7);
44+
StorageLive(_3);
45+
_3 = copy ((*_1).0: char);
46+
StorageLive(_4);
47+
_4 = copy ((*_2).0: char);
48+
_5 = Cmp(move _3, move _4);
49+
StorageDead(_4);
50+
StorageDead(_3);
51+
_6 = Option::<std::cmp::Ordering>::Some(copy _5);
52+
_7 = discriminant(_5);
53+
switchInt(move _7) -> [0: bb1, otherwise: bb2];
54+
}
55+
56+
bb1: {
57+
StorageLive(_10);
58+
StorageLive(_8);
59+
_8 = copy ((*_1).1: i16);
60+
StorageLive(_9);
61+
_9 = copy ((*_2).1: i16);
62+
_10 = Cmp(move _8, move _9);
63+
StorageDead(_9);
64+
StorageDead(_8);
65+
_11 = Option::<std::cmp::Ordering>::Some(move _10);
66+
StorageDead(_10);
67+
StorageDead(_7);
68+
StorageDead(_5);
69+
goto -> bb3;
70+
}
71+
72+
bb2: {
73+
_11 = copy _6;
74+
StorageDead(_7);
75+
StorageDead(_5);
76+
goto -> bb3;
77+
}
78+
79+
bb3: {
80+
_12 = move ((_11 as Some).0: std::cmp::Ordering);
81+
StorageLive(_13);
82+
_13 = discriminant(_12);
83+
_0 = Le(move _13, const 0_i8);
84+
StorageDead(_13);
85+
StorageDead(_11);
86+
StorageDead(_12);
87+
return;
88+
}
89+
}
+25-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,33 @@
1-
// skip-filecheck
21
//@ compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=0
32

43
#![crate_type = "lib"]
54

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

8+
// Because this isn't derived by the impl, it's not on the `{impl#0}-partial_cmp`,
9+
// and thus we need to call it to see what the inlined generic one produces.
10+
pub fn demo_le(a: &MultiField, b: &MultiField) -> bool {
11+
// CHECK-LABEL: fn demo_le
12+
// CHECK: inlined <MultiField as PartialOrd>::le
13+
// CHECK: inlined{{.+}}is_some_and
14+
// CHECK: inlined <MultiField as PartialOrd>::partial_cmp
15+
16+
// CHECK: [[A0:_[0-9]+]] = copy ((*_1).0: char);
17+
// CHECK: [[B0:_[0-9]+]] = copy ((*_2).0: char);
18+
// CHECK: Cmp(move [[A0]], move [[B0]]);
19+
20+
// CHECK: [[D0:_[0-9]+]] = discriminant({{.+}});
21+
// CHECK: switchInt(move [[D0]]) -> [0: bb{{[0-9]+}}, otherwise: bb{{[0-9]+}}];
22+
23+
// CHECK: [[A1:_[0-9]+]] = copy ((*_1).1: i16);
24+
// CHECK: [[B1:_[0-9]+]] = copy ((*_2).1: i16);
25+
// CHECK: Cmp(move [[A1]], move [[B1]]);
26+
27+
// CHECK: [[D1:_[0-9]+]] = discriminant({{.+}});
28+
// CHECK: _0 = Le(move [[D1]], const 0_i8);
29+
*a <= *b
30+
}
31+
932
// EMIT_MIR derived_ord.{impl#0}-partial_cmp.PreCodegen.after.mir
33+
// EMIT_MIR derived_ord.demo_le.PreCodegen.after.mir

tests/mir-opt/pre-codegen/derived_ord.{impl#0}-partial_cmp.PreCodegen.after.mir

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// MIR for `<impl at $DIR/derived_ord.rs:6:10: 6:20>::partial_cmp` after PreCodegen
1+
// MIR for `<impl at $DIR/derived_ord.rs:5:10: 5:20>::partial_cmp` after PreCodegen
22

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

0 commit comments

Comments
 (0)