Skip to content

Commit d6fa338

Browse files
scottmcmscottmcm
committed
Experiment: spaceship in MIR
As mentioned in <rust-lang#108788 (comment)>, `Ord::cmp` on primitives generates a large amount of MIR, preventing (or at least discouraging) it from mir-inlining. Let's see whether making it a MIR primitive helps stuff -- derived `(Partial)Ord` in particular, if we're lucky.
1 parent 7820b62 commit d6fa338

File tree

25 files changed

+266
-10
lines changed

25 files changed

+266
-10
lines changed

‎compiler/rustc_codegen_cranelift/src/codegen_i128.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ pub(crate) fn maybe_codegen<'tcx>(
148148
assert!(!checked);
149149
None
150150
}
151+
BinOp::Cmp => None,
151152
BinOp::Shl | BinOp::Shr => None,
152153
}
153154
}

‎compiler/rustc_codegen_cranelift/src/num.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,34 @@ pub(crate) fn bin_op_to_intcc(bin_op: BinOp, signed: bool) -> Option<IntCC> {
4040
})
4141
}
4242

43+
fn codegen_three_way_compare<'tcx>(
44+
fx: &mut FunctionCx<'_, '_, 'tcx>,
45+
signed: bool,
46+
lhs: Value,
47+
rhs: Value,
48+
) -> CValue<'tcx> {
49+
let gt_cc = crate::num::bin_op_to_intcc(BinOp::Gt, signed).unwrap();
50+
let lt_cc = crate::num::bin_op_to_intcc(BinOp::Lt, signed).unwrap();
51+
let gt = fx.bcx.ins().icmp(gt_cc, lhs, rhs);
52+
let lt = fx.bcx.ins().icmp(lt_cc, lhs, rhs);
53+
// Cranelift no longer has a single-bit type, so the comparison results
54+
// are already `I8`s, which we can subtract to get the -1/0/+1 we want.
55+
// See <https://github.com/bytecodealliance/wasmtime/pull/5031>.
56+
let val = fx.bcx.ins().isub(gt, lt);
57+
CValue::by_val(val, fx.layout_of(fx.tcx.types.i8))
58+
}
59+
4360
fn codegen_compare_bin_op<'tcx>(
4461
fx: &mut FunctionCx<'_, '_, 'tcx>,
4562
bin_op: BinOp,
4663
signed: bool,
4764
lhs: Value,
4865
rhs: Value,
4966
) -> CValue<'tcx> {
67+
if bin_op == BinOp::Cmp {
68+
return codegen_three_way_compare(fx, signed, lhs, rhs);
69+
}
70+
5071
let intcc = crate::num::bin_op_to_intcc(bin_op, signed).unwrap();
5172
let val = fx.bcx.ins().icmp(intcc, lhs, rhs);
5273
CValue::by_val(val, fx.layout_of(fx.tcx.types.bool))
@@ -59,7 +80,7 @@ pub(crate) fn codegen_binop<'tcx>(
5980
in_rhs: CValue<'tcx>,
6081
) -> CValue<'tcx> {
6182
match bin_op {
62-
BinOp::Eq | BinOp::Lt | BinOp::Le | BinOp::Ne | BinOp::Ge | BinOp::Gt => {
83+
BinOp::Eq | BinOp::Lt | BinOp::Le | BinOp::Ne | BinOp::Ge | BinOp::Gt | BinOp::Cmp => {
6384
match in_lhs.layout().ty.kind() {
6485
ty::Bool | ty::Uint(_) | ty::Int(_) | ty::Char => {
6586
let signed = type_sign(in_lhs.layout().ty);

‎compiler/rustc_codegen_gcc/src/common.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
9797
self.const_int(self.type_i32(), i as i64)
9898
}
9999

100+
fn const_i8(&self, i: i8) -> RValue<'gcc> {
101+
self.const_int(self.type_i8(), i as i64)
102+
}
103+
100104
fn const_u32(&self, i: u32) -> RValue<'gcc> {
101105
self.const_uint(self.type_u32(), i as u64)
102106
}

‎compiler/rustc_codegen_llvm/src/common.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
157157
self.const_int(self.type_i32(), i as i64)
158158
}
159159

160+
fn const_i8(&self, i: i8) -> &'ll Value {
161+
self.const_int(self.type_i8(), i as i64)
162+
}
163+
160164
fn const_u32(&self, i: u32) -> &'ll Value {
161165
self.const_uint(self.type_i32(), i as u64)
162166
}

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::common::{self, IntPredicate};
77
use crate::traits::*;
88
use crate::MemFlags;
99

10+
use rustc_hir as hir;
1011
use rustc_middle::mir;
1112
use rustc_middle::mir::Operand;
1213
use rustc_middle::ty::cast::{CastTy, IntTy};
@@ -599,6 +600,27 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
599600
bx.icmp(base::bin_op_to_icmp_predicate(op.to_hir_binop(), is_signed), lhs, rhs)
600601
}
601602
}
603+
mir::BinOp::Cmp => {
604+
use std::cmp::Ordering;
605+
debug_assert!(!is_float);
606+
// FIXME: To avoid this PR changing behaviour, the operations used
607+
// here are those from <https://github.com/rust-lang/rust/pull/63767>,
608+
// as tested by `tests/codegen/integer-cmp.rs`.
609+
// Something in future might want to pick different ones. For example,
610+
// maybe the ones from Clang's `<=>` operator in C++20 (see
611+
// <https://github.com/llvm/llvm-project/issues/60012>) or once we
612+
// update to new LLVM, something to take advantage of the new folds in
613+
// <https://github.com/llvm/llvm-project/issues/59666>.
614+
let pred = |op| base::bin_op_to_icmp_predicate(op, is_signed);
615+
let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs);
616+
let is_ne = bx.icmp(pred(hir::BinOpKind::Ne), lhs, rhs);
617+
let ge = bx.select(
618+
is_ne,
619+
bx.cx().const_i8(Ordering::Greater as i8),
620+
bx.cx().const_i8(Ordering::Equal as i8),
621+
);
622+
bx.select(is_lt, bx.cx().const_i8(Ordering::Less as i8), ge)
623+
}
602624
}
603625
}
604626

‎compiler/rustc_codegen_ssa/src/traits/consts.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub trait ConstMethods<'tcx>: BackendTypes {
1414
fn const_bool(&self, val: bool) -> Self::Value;
1515
fn const_i16(&self, i: i16) -> Self::Value;
1616
fn const_i32(&self, i: i32) -> Self::Value;
17+
fn const_i8(&self, i: i8) -> Self::Value;
1718
fn const_u32(&self, i: u32) -> Self::Value;
1819
fn const_u64(&self, i: u64) -> Self::Value;
1920
fn const_usize(&self, i: u64) -> Self::Value;

‎compiler/rustc_const_eval/src/interpret/operator.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
5959
}
6060

6161
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
62+
fn three_way_compare<T: Ord>(&self, lhs: T, rhs: T) -> (Scalar<M::Provenance>, bool, Ty<'tcx>) {
63+
let res = Ord::cmp(&lhs, &rhs) as i8;
64+
return (Scalar::from_i8(res), false, self.tcx.ty_ordering_enum(None));
65+
}
66+
6267
fn binary_char_op(
6368
&self,
6469
bin_op: mir::BinOp,
@@ -67,6 +72,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
6772
) -> (Scalar<M::Provenance>, bool, Ty<'tcx>) {
6873
use rustc_middle::mir::BinOp::*;
6974

75+
if bin_op == Cmp {
76+
return self.three_way_compare(l, r);
77+
}
78+
7079
let res = match bin_op {
7180
Eq => l == r,
7281
Ne => l != r,
@@ -211,6 +220,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
211220
let r = self.sign_extend(r, right_layout) as i128;
212221
return Ok((Scalar::from_bool(op(&l, &r)), false, self.tcx.types.bool));
213222
}
223+
if bin_op == Cmp {
224+
let l = self.sign_extend(l, left_layout) as i128;
225+
let r = self.sign_extend(r, right_layout) as i128;
226+
return Ok(self.three_way_compare(l, r));
227+
}
214228
let op: Option<fn(i128, i128) -> (i128, bool)> = match bin_op {
215229
Div if r == 0 => throw_ub!(DivisionByZero),
216230
Rem if r == 0 => throw_ub!(RemainderByZero),
@@ -250,6 +264,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
250264
}
251265
}
252266

267+
if bin_op == Cmp {
268+
return Ok(self.three_way_compare(l, r));
269+
}
270+
253271
let (val, ty) = match bin_op {
254272
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
255273
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),

‎compiler/rustc_const_eval/src/interpret/step.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ fn binop_left_homogeneous(op: mir::BinOp) -> bool {
1717
use rustc_middle::mir::BinOp::*;
1818
match op {
1919
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Offset | Shl | Shr => true,
20-
Eq | Ne | Lt | Le | Gt | Ge => false,
20+
Eq | Ne | Lt | Le | Gt | Ge | Cmp => false,
2121
}
2222
}
2323
/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the
@@ -26,7 +26,8 @@ fn binop_left_homogeneous(op: mir::BinOp) -> bool {
2626
fn binop_right_homogeneous(op: mir::BinOp) -> bool {
2727
use rustc_middle::mir::BinOp::*;
2828
match op {
29-
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true,
29+
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge
30+
| Cmp => true,
3031
Offset | Shl | Shr => false,
3132
}
3233
}

‎compiler/rustc_const_eval/src/transform/promote_consts.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@ impl<'tcx> Validator<'_, 'tcx> {
568568
| BinOp::Lt
569569
| BinOp::Ge
570570
| BinOp::Gt
571+
| BinOp::Cmp
571572
| BinOp::Offset
572573
| BinOp::Add
573574
| BinOp::Sub

‎compiler/rustc_const_eval/src/transform/validate.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
483483
);
484484
}
485485
}
486+
Cmp => {
487+
for x in [a, b] {
488+
check_kinds!(
489+
x,
490+
"Cannot three-way compare non-integer type {:?}",
491+
ty::Char | ty::Uint(..) | ty::Int(..)
492+
)
493+
}
494+
}
486495
Shl | Shr => {
487496
for x in [a, b] {
488497
check_kinds!(

‎compiler/rustc_hir/src/lang_items.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ language_item_table! {
221221
Unpin, sym::unpin, unpin_trait, Target::Trait, GenericRequirement::None;
222222
Pin, sym::pin, pin_type, Target::Struct, GenericRequirement::None;
223223

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

‎compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir
9999
| sym::cttz
100100
| sym::bswap
101101
| sym::bitreverse
102+
| sym::three_way_compare
102103
| sym::discriminant_value
103104
| sym::type_id
104105
| sym::likely
@@ -316,6 +317,10 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
316317
| sym::bswap
317318
| sym::bitreverse => (1, vec![param(0)], param(0)),
318319

320+
sym::three_way_compare => {
321+
(1, vec![param(0), param(0)], tcx.ty_ordering_enum(Some(it.span)))
322+
}
323+
319324
sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
320325
(1, vec![param(0), param(0)], tcx.mk_tup(&[param(0), tcx.types.bool]))
321326
}

‎compiler/rustc_middle/src/mir/interpret/value.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ impl<Prov> Scalar<Prov> {
257257
.unwrap_or_else(|| bug!("Signed value {:#x} does not fit in {} bits", i, size.bits()))
258258
}
259259

260+
#[inline]
261+
pub fn from_i8(i: i8) -> Self {
262+
Self::from_int(i, Size::from_bits(8))
263+
}
264+
260265
#[inline]
261266
pub fn from_i32(i: i32) -> Self {
262267
Self::from_int(i, Size::from_bits(32))

‎compiler/rustc_middle/src/mir/syntax.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,8 @@ pub enum BinOp {
12711271
Ge,
12721272
/// The `>` operator (greater than)
12731273
Gt,
1274+
/// The `<=>` operator (three-way comparison, like `Ord::cmp`)
1275+
Cmp,
12741276
/// The `ptr.offset` operator
12751277
Offset,
12761278
}

‎compiler/rustc_middle/src/mir/tcx.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,11 @@ impl<'tcx> BinOp {
260260
&BinOp::Eq | &BinOp::Lt | &BinOp::Le | &BinOp::Ne | &BinOp::Ge | &BinOp::Gt => {
261261
tcx.types.bool
262262
}
263+
&BinOp::Cmp => {
264+
// these should be integer-like types of the same size.
265+
assert_eq!(lhs_ty, rhs_ty);
266+
tcx.ty_ordering_enum(None)
267+
}
263268
}
264269
}
265270
}
@@ -301,6 +306,7 @@ impl BinOp {
301306
BinOp::Gt => hir::BinOpKind::Gt,
302307
BinOp::Le => hir::BinOpKind::Le,
303308
BinOp::Ge => hir::BinOpKind::Ge,
309+
BinOp::Cmp => unreachable!(),
304310
BinOp::Offset => unreachable!(),
305311
}
306312
}

‎compiler/rustc_middle/src/ty/context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,13 @@ impl<'tcx> TyCtxt<'tcx> {
716716
}
717717
}
718718

719+
/// Gets a `Ty` representing the [`LangItem::OrderingEnum`]
720+
#[track_caller]
721+
pub fn ty_ordering_enum(self, span: Option<Span>) -> Ty<'tcx> {
722+
let ordering_enum = self.require_lang_item(hir::LangItem::OrderingEnum, span);
723+
self.type_of(ordering_enum).no_bound_vars().unwrap()
724+
}
725+
719726
/// Constructs a `TyKind::Error` type with current `ErrorGuaranteed`
720727
#[track_caller]
721728
pub fn ty_error(self, reported: ErrorGuaranteed) -> Ty<'tcx> {

‎compiler/rustc_mir_transform/src/lower_intrinsics.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
8181
drop(args);
8282
terminator.kind = TerminatorKind::Goto { target };
8383
}
84-
sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => {
84+
sym::wrapping_add
85+
| sym::wrapping_sub
86+
| sym::wrapping_mul
87+
| sym::three_way_compare => {
8588
if let Some(target) = *target {
8689
let lhs;
8790
let rhs;
@@ -94,6 +97,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
9497
sym::wrapping_add => BinOp::Add,
9598
sym::wrapping_sub => BinOp::Sub,
9699
sym::wrapping_mul => BinOp::Mul,
100+
sym::three_way_compare => BinOp::Cmp,
97101
_ => bug!("unexpected intrinsic"),
98102
};
99103
block.statements.push(Statement {

‎compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,7 @@ symbols! {
14731473
thread,
14741474
thread_local,
14751475
thread_local_macro,
1476+
three_way_compare,
14761477
thumb2,
14771478
thumb_mode: "thumb-mode",
14781479
tmm_reg,

‎compiler/rustc_ty_utils/src/consts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn check_binop(op: mir::BinOp) -> bool {
7878
use mir::BinOp::*;
7979
match op {
8080
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Shl | Shr | Eq | Lt | Le | Ne
81-
| Ge | Gt => true,
81+
| Ge | Gt | Cmp => true,
8282
Offset => false,
8383
}
8484
}

‎library/core/src/cmp.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ pub struct AssertParamIsEq<T: Eq + ?Sized> {
335335
#[derive(Clone, Copy, Eq, Debug, Hash)]
336336
#[derive_const(PartialOrd, Ord, PartialEq)]
337337
#[stable(feature = "rust1", since = "1.0.0")]
338+
#[cfg_attr(not(bootstrap), lang = "Ordering")]
338339
#[repr(i8)]
339340
pub enum Ordering {
340341
/// An ordering where a compared value is less than another.
@@ -1410,11 +1411,18 @@ mod impls {
14101411
impl const Ord for $t {
14111412
#[inline]
14121413
fn cmp(&self, other: &$t) -> Ordering {
1413-
// The order here is important to generate more optimal assembly.
1414-
// See <https://github.com/rust-lang/rust/issues/63758> for more info.
1415-
if *self < *other { Less }
1416-
else if *self == *other { Equal }
1417-
else { Greater }
1414+
#[cfg(bootstrap)]
1415+
{
1416+
// The order here is important to generate more optimal assembly.
1417+
// See <https://github.com/rust-lang/rust/issues/63758> for more info.
1418+
if *self < *other { Less }
1419+
else if *self == *other { Equal }
1420+
else { Greater }
1421+
}
1422+
#[cfg(not(bootstrap))]
1423+
{
1424+
crate::intrinsics::three_way_compare(*self, *other)
1425+
}
14181426
}
14191427
}
14201428
)*)

‎library/core/src/intrinsics.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,6 +1802,18 @@ extern "rust-intrinsic" {
18021802
#[rustc_safe_intrinsic]
18031803
pub fn bitreverse<T: Copy>(x: T) -> T;
18041804

1805+
/// Does a three-way comparison between the two integer arguments.
1806+
///
1807+
/// This is included as an intrinsic as it's useful to let it be one thing
1808+
/// in MIR, rather than the multiple checks and switches that make its IR
1809+
/// large and difficult to optimize.
1810+
///
1811+
/// The stabilized version of this intrinsic is [`Ord::cmp`].
1812+
#[cfg(not(bootstrap))]
1813+
#[rustc_const_unstable(feature = "const_cmp", issue = "92391")]
1814+
#[rustc_safe_intrinsic]
1815+
pub fn three_way_compare<T: Copy>(lhs: T, rhs: T) -> crate::cmp::Ordering;
1816+
18051817
/// Performs checked integer addition.
18061818
///
18071819
/// Note that, unlike most intrinsics, this is safe to call;

‎tests/mir-opt/lower_intrinsics.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,18 @@ pub fn with_overflow(a: i32, b: i32) {
7979
let _y = core::intrinsics::sub_with_overflow(a, b);
8080
let _z = core::intrinsics::mul_with_overflow(a, b);
8181
}
82+
83+
// EMIT_MIR lower_intrinsics.three_way_compare_char.LowerIntrinsics.diff
84+
pub fn three_way_compare_char(a: char, b: char) {
85+
let _x = core::intrinsics::three_way_compare(a, b);
86+
}
87+
88+
// EMIT_MIR lower_intrinsics.three_way_compare_signed.LowerIntrinsics.diff
89+
pub fn three_way_compare_signed(a: i16, b: i16) {
90+
core::intrinsics::three_way_compare(a, b);
91+
}
92+
93+
// EMIT_MIR lower_intrinsics.three_way_compare_unsigned.LowerIntrinsics.diff
94+
pub fn three_way_compare_unsigned(a: u32, b: u32) {
95+
let _x = core::intrinsics::three_way_compare(a, b);
96+
}

0 commit comments

Comments
 (0)