Skip to content

Commit

Permalink
Rollup merge of rust-lang#107022 - scottmcm:ordering-option-eq, r=m-o…
Browse files Browse the repository at this point in the history
…u-se

Implement `SpecOptionPartialEq` for `cmp::Ordering`

Noticed as I continue to explore options for having code using `partial_cmp` optimize better.

Before:
```llvm
; Function Attrs: mustprogress nofree nosync nounwind willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #0 {
start:
  %2 = icmp eq i8 %0, 2
  br i1 %2, label %bb1.i, label %bb3.i

bb1.i:                                            ; preds = %start
  %3 = icmp eq i8 %1, 2
  br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"

bb3.i:                                            ; preds = %start
  %.not.i = icmp ne i8 %1, 2
  %4 = icmp eq i8 %0, %1
  %spec.select.i = and i1 %.not.i, %4
  br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"

"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit": ; preds = %bb1.i, %bb3.i
  %.0.i = phi i1 [ %3, %bb1.i ], [ %spec.select.i, %bb3.i ]
  ret i1 %.0.i
}
```

After:
```llvm
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #1 {
start:
  %2 = icmp eq i8 %0, %1
  ret i1 %2
}
```

(Which <https://alive2.llvm.org/ce/z/-rop5r> says LLVM *could* just do itself, but there's probably an issue already open for that problem from when this was originally looked at for `Option<NonZeroU8>` and friends.)
  • Loading branch information
matthiaskrgr authored Jan 28, 2023
2 parents 6cd6bad + 3e9d1e4 commit 7b78b6a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
16 changes: 15 additions & 1 deletion library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ use crate::marker::Destruct;
use crate::panicking::{panic, panic_str};
use crate::pin::Pin;
use crate::{
convert, hint, mem,
cmp, convert, hint, mem,
ops::{self, ControlFlow, Deref, DerefMut},
};

Expand Down Expand Up @@ -2090,6 +2090,12 @@ impl<T: PartialEq> PartialEq for Option<T> {
}
}

/// This specialization trait is a workaround for LLVM not currently (2023-01)
/// being able to optimize this itself, even though Alive confirms that it would
/// be legal to do so: <https://github.com/llvm/llvm-project/issues/52622>
///
/// Once that's fixed, `Option` should go back to deriving `PartialEq`, as
/// it used to do before <https://github.com/rust-lang/rust/pull/103556>.
#[unstable(feature = "spec_option_partial_eq", issue = "none", reason = "exposed only for rustc")]
#[doc(hidden)]
pub trait SpecOptionPartialEq: Sized {
Expand Down Expand Up @@ -2146,6 +2152,14 @@ impl<T> SpecOptionPartialEq for crate::ptr::NonNull<T> {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl SpecOptionPartialEq for cmp::Ordering {
#[inline]
fn eq(l: &Option<Self>, r: &Option<Self>) -> bool {
l.map_or(2, |x| x as i8) == r.map_or(2, |x| x as i8)
}
}

/////////////////////////////////////////////////////////////////////////////
// The Option Iterators
/////////////////////////////////////////////////////////////////////////////
Expand Down
10 changes: 10 additions & 0 deletions tests/codegen/option-nonzero-eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![crate_type = "lib"]

extern crate core;
use core::cmp::Ordering;
use core::num::{NonZeroU32, NonZeroI64};
use core::ptr::NonNull;

Expand Down Expand Up @@ -32,3 +33,12 @@ pub fn non_null_eq(l: Option<NonNull<u8>>, r: Option<NonNull<u8>>) -> bool {
// CHECK-NEXT: ret i1
l == r
}

// CHECK-lABEL: @ordering_eq
#[no_mangle]
pub fn ordering_eq(l: Option<Ordering>, r: Option<Ordering>) -> bool {
// CHECK: start:
// CHECK-NEXT: icmp eq i8
// CHECK-NEXT: ret i1
l == r
}

0 comments on commit 7b78b6a

Please sign in to comment.