Skip to content

Experiment: spaceship in MIR #108800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/codegen_i128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ pub(crate) fn maybe_codegen<'tcx>(
assert!(!checked);
None
}
BinOp::Cmp => None,
BinOp::Shl | BinOp::Shr => None,
}
}
23 changes: 22 additions & 1 deletion compiler/rustc_codegen_cranelift/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,34 @@ pub(crate) fn bin_op_to_intcc(bin_op: BinOp, signed: bool) -> Option<IntCC> {
})
}

fn codegen_three_way_compare<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
signed: bool,
lhs: Value,
rhs: Value,
) -> CValue<'tcx> {
let gt_cc = crate::num::bin_op_to_intcc(BinOp::Gt, signed).unwrap();
let lt_cc = crate::num::bin_op_to_intcc(BinOp::Lt, signed).unwrap();
let gt = fx.bcx.ins().icmp(gt_cc, lhs, rhs);
let lt = fx.bcx.ins().icmp(lt_cc, lhs, rhs);
// Cranelift no longer has a single-bit type, so the comparison results
// are already `I8`s, which we can subtract to get the -1/0/+1 we want.
// See <https://github.com/bytecodealliance/wasmtime/pull/5031>.
let val = fx.bcx.ins().isub(gt, lt);
CValue::by_val(val, fx.layout_of(fx.tcx.types.i8))
}

fn codegen_compare_bin_op<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
bin_op: BinOp,
signed: bool,
lhs: Value,
rhs: Value,
) -> CValue<'tcx> {
if bin_op == BinOp::Cmp {
return codegen_three_way_compare(fx, signed, lhs, rhs);
}

let intcc = crate::num::bin_op_to_intcc(bin_op, signed).unwrap();
let val = fx.bcx.ins().icmp(intcc, lhs, rhs);
CValue::by_val(val, fx.layout_of(fx.tcx.types.bool))
Expand All @@ -59,7 +80,7 @@ pub(crate) fn codegen_binop<'tcx>(
in_rhs: CValue<'tcx>,
) -> CValue<'tcx> {
match bin_op {
BinOp::Eq | BinOp::Lt | BinOp::Le | BinOp::Ne | BinOp::Ge | BinOp::Gt => {
BinOp::Eq | BinOp::Lt | BinOp::Le | BinOp::Ne | BinOp::Ge | BinOp::Gt | BinOp::Cmp => {
match in_lhs.layout().ty.kind() {
ty::Bool | ty::Uint(_) | ty::Int(_) | ty::Char => {
let signed = type_sign(in_lhs.layout().ty);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
self.const_int(self.type_i32(), i as i64)
}

fn const_i8(&self, i: i8) -> RValue<'gcc> {
self.const_int(self.type_i8(), i as i64)
}

fn const_u32(&self, i: u32) -> RValue<'gcc> {
self.const_uint(self.type_u32(), i as u64)
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
self.const_int(self.type_i32(), i as i64)
}

fn const_i8(&self, i: i8) -> &'ll Value {
self.const_int(self.type_i8(), i as i64)
}

fn const_u32(&self, i: u32) -> &'ll Value {
self.const_uint(self.type_i32(), i as u64)
}
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::common::{self, IntPredicate};
use crate::traits::*;
use crate::MemFlags;

use rustc_hir as hir;
use rustc_middle::mir;
use rustc_middle::mir::Operand;
use rustc_middle::ty::cast::{CastTy, IntTy};
Expand Down Expand Up @@ -599,6 +600,27 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.icmp(base::bin_op_to_icmp_predicate(op.to_hir_binop(), is_signed), lhs, rhs)
}
}
mir::BinOp::Cmp => {
use std::cmp::Ordering;
debug_assert!(!is_float);
// FIXME: To avoid this PR changing behaviour, the operations used
// here are those from <https://github.com/rust-lang/rust/pull/63767>,
// as tested by `tests/codegen/integer-cmp.rs`.
// Something in future might want to pick different ones. For example,
// maybe the ones from Clang's `<=>` operator in C++20 (see
// <https://github.com/llvm/llvm-project/issues/60012>) or once we
// update to new LLVM, something to take advantage of the new folds in
// <https://github.com/llvm/llvm-project/issues/59666>.
let pred = |op| base::bin_op_to_icmp_predicate(op, is_signed);
let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs);
let is_ne = bx.icmp(pred(hir::BinOpKind::Ne), lhs, rhs);
let ge = bx.select(
is_ne,
bx.cx().const_i8(Ordering::Greater as i8),
bx.cx().const_i8(Ordering::Equal as i8),
);
bx.select(is_lt, bx.cx().const_i8(Ordering::Less as i8), ge)
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub trait ConstMethods<'tcx>: BackendTypes {
fn const_bool(&self, val: bool) -> Self::Value;
fn const_i16(&self, i: i16) -> Self::Value;
fn const_i32(&self, i: i32) -> Self::Value;
fn const_i8(&self, i: i8) -> Self::Value;
fn const_u32(&self, i: u32) -> Self::Value;
fn const_u64(&self, i: u64) -> Self::Value;
fn const_usize(&self, i: u64) -> Self::Value;
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
fn three_way_compare<T: Ord>(&self, lhs: T, rhs: T) -> (Scalar<M::Provenance>, bool, Ty<'tcx>) {
let res = Ord::cmp(&lhs, &rhs) as i8;
return (Scalar::from_i8(res), false, self.tcx.ty_ordering_enum(None));
}

fn binary_char_op(
&self,
bin_op: mir::BinOp,
Expand All @@ -67,6 +72,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> (Scalar<M::Provenance>, bool, Ty<'tcx>) {
use rustc_middle::mir::BinOp::*;

if bin_op == Cmp {
return self.three_way_compare(l, r);
}

let res = match bin_op {
Eq => l == r,
Ne => l != r,
Expand Down Expand Up @@ -211,6 +220,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let r = self.sign_extend(r, right_layout) as i128;
return Ok((Scalar::from_bool(op(&l, &r)), false, self.tcx.types.bool));
}
if bin_op == Cmp {
let l = self.sign_extend(l, left_layout) as i128;
let r = self.sign_extend(r, right_layout) as i128;
return Ok(self.three_way_compare(l, r));
}
let op: Option<fn(i128, i128) -> (i128, bool)> = match bin_op {
Div if r == 0 => throw_ub!(DivisionByZero),
Rem if r == 0 => throw_ub!(RemainderByZero),
Expand Down Expand Up @@ -250,6 +264,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

if bin_op == Cmp {
return Ok(self.three_way_compare(l, r));
}

let (val, ty) = match bin_op {
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn binop_left_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Offset | Shl | Shr => true,
Eq | Ne | Lt | Le | Gt | Ge => false,
Eq | Ne | Lt | Le | Gt | Ge | Cmp => false,
}
}
/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the
Expand All @@ -26,7 +26,8 @@ fn binop_left_homogeneous(op: mir::BinOp) -> bool {
fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true,
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge
| Cmp => true,
Offset | Shl | Shr => false,
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ impl<'tcx> Validator<'_, 'tcx> {
| BinOp::Lt
| BinOp::Ge
| BinOp::Gt
| BinOp::Cmp
| BinOp::Offset
| BinOp::Add
| BinOp::Sub
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
Cmp => {
for x in [a, b] {
check_kinds!(
x,
"Cannot three-way compare non-integer type {:?}",
ty::Char | ty::Uint(..) | ty::Int(..)
)
}
}
Shl | Shr => {
for x in [a, b] {
check_kinds!(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ language_item_table! {
Unpin, sym::unpin, unpin_trait, Target::Trait, GenericRequirement::None;
Pin, sym::pin, pin_type, Target::Struct, GenericRequirement::None;

OrderingEnum, sym::Ordering, ordering_enum, Target::Enum, GenericRequirement::Exact(0);
PartialEq, sym::eq, eq_trait, Target::Trait, GenericRequirement::Exact(1);
PartialOrd, sym::partial_ord, partial_ord_trait, Target::Trait, GenericRequirement::Exact(1);

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir
| sym::cttz
| sym::bswap
| sym::bitreverse
| sym::three_way_compare
| sym::discriminant_value
| sym::type_id
| sym::likely
Expand Down Expand Up @@ -316,6 +317,10 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
| sym::bswap
| sym::bitreverse => (1, vec![param(0)], param(0)),

sym::three_way_compare => {
(1, vec![param(0), param(0)], tcx.ty_ordering_enum(Some(it.span)))
}

sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
(1, vec![param(0), param(0)], tcx.mk_tup(&[param(0), tcx.types.bool]))
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ impl<Prov> Scalar<Prov> {
.unwrap_or_else(|| bug!("Signed value {:#x} does not fit in {} bits", i, size.bits()))
}

#[inline]
pub fn from_i8(i: i8) -> Self {
Self::from_int(i, Size::from_bits(8))
}

#[inline]
pub fn from_i32(i: i32) -> Self {
Self::from_int(i, Size::from_bits(32))
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,8 @@ pub enum BinOp {
Ge,
/// The `>` operator (greater than)
Gt,
/// The `<=>` operator (three-way comparison, like `Ord::cmp`)
Cmp,
/// The `ptr.offset` operator
Offset,
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ impl<'tcx> BinOp {
&BinOp::Eq | &BinOp::Lt | &BinOp::Le | &BinOp::Ne | &BinOp::Ge | &BinOp::Gt => {
tcx.types.bool
}
&BinOp::Cmp => {
// these should be integer-like types of the same size.
assert_eq!(lhs_ty, rhs_ty);
tcx.ty_ordering_enum(None)
}
}
}
}
Expand Down Expand Up @@ -301,6 +306,7 @@ impl BinOp {
BinOp::Gt => hir::BinOpKind::Gt,
BinOp::Le => hir::BinOpKind::Le,
BinOp::Ge => hir::BinOpKind::Ge,
BinOp::Cmp => unreachable!(),
BinOp::Offset => unreachable!(),
}
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,13 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// Gets a `Ty` representing the [`LangItem::OrderingEnum`]
#[track_caller]
pub fn ty_ordering_enum(self, span: Option<Span>) -> Ty<'tcx> {
let ordering_enum = self.require_lang_item(hir::LangItem::OrderingEnum, span);
self.type_of(ordering_enum).no_bound_vars().unwrap()
}

/// Constructs a `TyKind::Error` type with current `ErrorGuaranteed`
#[track_caller]
pub fn ty_error(self, reported: ErrorGuaranteed) -> Ty<'tcx> {
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_mir_transform/src/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
drop(args);
terminator.kind = TerminatorKind::Goto { target };
}
sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => {
sym::wrapping_add
| sym::wrapping_sub
| sym::wrapping_mul
| sym::three_way_compare => {
if let Some(target) = *target {
let lhs;
let rhs;
Expand All @@ -94,6 +97,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
sym::wrapping_add => BinOp::Add,
sym::wrapping_sub => BinOp::Sub,
sym::wrapping_mul => BinOp::Mul,
sym::three_way_compare => BinOp::Cmp,
_ => bug!("unexpected intrinsic"),
};
block.statements.push(Statement {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,7 @@ symbols! {
thread,
thread_local,
thread_local_macro,
three_way_compare,
thumb2,
thumb_mode: "thumb-mode",
tmm_reg,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn check_binop(op: mir::BinOp) -> bool {
use mir::BinOp::*;
match op {
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Shl | Shr | Eq | Lt | Le | Ne
| Ge | Gt => true,
| Ge | Gt | Cmp => true,
Offset => false,
}
}
Expand Down
18 changes: 13 additions & 5 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ pub struct AssertParamIsEq<T: Eq + ?Sized> {
#[derive(Clone, Copy, Eq, Debug, Hash)]
#[derive_const(PartialOrd, Ord, PartialEq)]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(bootstrap), lang = "Ordering")]
#[repr(i8)]
pub enum Ordering {
/// An ordering where a compared value is less than another.
Expand Down Expand Up @@ -1410,11 +1411,18 @@ mod impls {
impl const 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 }
#[cfg(bootstrap)]
{
// 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 }
}
#[cfg(not(bootstrap))]
{
crate::intrinsics::three_way_compare(*self, *other)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a question for my own understanding: is there a reason why it's better (or has to be) for this to be in a single instance of the fn cmp rather than either #[cfg(bootstrap)] fn cmp(...) or on the impl?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just put it here because it's the least copy-pasting. Doing it at other levels would work too, but would mean duplicating the #[inline] or stability attributes, which leaves more opportunities for me to get it wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have the duplicate the signature and the #[inline] attribute. But it doesn't make that much of a difference (also hopefully we'll get rid of these cfg(bootstrap) inside the standard library soon anyways🤞)

}
}
}
)*)
Expand Down
12 changes: 12 additions & 0 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,18 @@ extern "rust-intrinsic" {
#[rustc_safe_intrinsic]
pub fn bitreverse<T: Copy>(x: T) -> T;

/// Does a three-way comparison between the two integer arguments.
///
/// This is included as an intrinsic as it's useful to let it be one thing
/// in MIR, rather than the multiple checks and switches that make its IR
/// large and difficult to optimize.
///
/// The stabilized version of this intrinsic is [`Ord::cmp`].
#[cfg(not(bootstrap))]
#[rustc_const_unstable(feature = "const_cmp", issue = "92391")]
#[rustc_safe_intrinsic]
pub fn three_way_compare<T: Copy>(lhs: T, rhs: T) -> crate::cmp::Ordering;

/// Performs checked integer addition.
///
/// Note that, unlike most intrinsics, this is safe to call;
Expand Down
15 changes: 15 additions & 0 deletions tests/mir-opt/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,18 @@ pub fn with_overflow(a: i32, b: i32) {
let _y = core::intrinsics::sub_with_overflow(a, b);
let _z = core::intrinsics::mul_with_overflow(a, b);
}

// EMIT_MIR lower_intrinsics.three_way_compare_char.LowerIntrinsics.diff
pub fn three_way_compare_char(a: char, b: char) {
let _x = core::intrinsics::three_way_compare(a, b);
}

// EMIT_MIR lower_intrinsics.three_way_compare_signed.LowerIntrinsics.diff
pub fn three_way_compare_signed(a: i16, b: i16) {
core::intrinsics::three_way_compare(a, b);
}

// EMIT_MIR lower_intrinsics.three_way_compare_unsigned.LowerIntrinsics.diff
pub fn three_way_compare_unsigned(a: u32, b: u32) {
let _x = core::intrinsics::three_way_compare(a, b);
}
Loading