Skip to content

Commit

Permalink
Add mir::BinOp::Cmp
Browse files Browse the repository at this point in the history
  • Loading branch information
scottmcm committed Nov 26, 2023
1 parent 7b56ce0 commit 8811efa
Show file tree
Hide file tree
Showing 35 changed files with 568 additions and 221 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_cranelift/src/codegen_i128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(crate) fn maybe_codegen<'tcx>(
Some(CValue::by_val(ret_val, lhs.layout()))
}
}
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne => None,
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne | BinOp::Cmp => None,
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => None,
}
}
Expand Down Expand Up @@ -134,6 +134,7 @@ pub(crate) fn maybe_codegen_checked<'tcx>(
BinOp::AddUnchecked | BinOp::SubUnchecked | BinOp::MulUnchecked => unreachable!(),
BinOp::Offset => unreachable!("offset should only be used on pointers, not 128bit ints"),
BinOp::Div | BinOp::Rem => unreachable!(),
BinOp::Cmp => unreachable!(),
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne => unreachable!(),
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => unreachable!(),
}
Expand Down
25 changes: 23 additions & 2 deletions 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 Expand Up @@ -160,7 +181,7 @@ pub(crate) fn codegen_int_binop<'tcx>(
}
BinOp::Offset => unreachable!("Offset is not an integer operation"),
// Compare binops handles by `codegen_binop`.
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge => {
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge | BinOp::Cmp => {
unreachable!("{:?}({:?}, {:?})", bin_op, in_lhs.layout().ty, in_rhs.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 @@ -104,6 +104,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 @@ -158,6 +158,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 @@ -881,6 +882,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 @@ -19,6 +19,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_u128(&self, i: u128) -> Self::Value;
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
Self::from_scalar(Scalar::from_bool(b), layout)
}

#[inline]
pub fn from_ordering(c: std::cmp::Ordering, tcx: TyCtxt<'tcx>) -> Self {
let ty = tcx.ty_ordering_enum(None);
let layout = tcx.layout_of(ty::ParamEnv::reveal_all().and(ty)).unwrap();
Self::from_scalar(Scalar::from_i8(c as i8), layout)
}

#[inline]
pub fn to_const_int(self) -> ConstInt {
assert!(self.layout.ty.is_integral());
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 @@ -61,6 +61,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) -> (ImmTy<'tcx, M::Provenance>, bool) {
let res = Ord::cmp(&lhs, &rhs);
return (ImmTy::from_ordering(res, *self.tcx), false);
}

fn binary_char_op(
&self,
bin_op: mir::BinOp,
Expand All @@ -69,6 +74,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> (ImmTy<'tcx, M::Provenance>, bool) {
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 @@ -231,6 +240,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((ImmTy::from_bool(op(&l, &r), *self.tcx), false));
}
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 @@ -270,6 +284,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 = match bin_op {
Eq => ImmTy::from_bool(l == r, *self.tcx),
Ne => ImmTy::from_bool(l != r, *self.tcx),
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 @@ -573,6 +573,7 @@ impl<'tcx> Validator<'_, 'tcx> {
| BinOp::Lt
| BinOp::Ge
| BinOp::Gt
| BinOp::Cmp
| BinOp::Offset
| BinOp::Add
| BinOp::AddUnchecked
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 @@ -932,6 +932,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(..)
)
}
}
AddUnchecked | SubUnchecked | MulUnchecked | Shl | ShlUnchecked | Shr
| ShrUnchecked => {
for x in [a, b] {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool {
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true,
Eq | Ne | Lt | Le | Gt | Ge => false,
Eq | Ne | Lt | Le | Gt | Ge | Cmp => false,
}
}

Expand All @@ -30,7 +30,7 @@ pub(crate) fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true,
| BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge | Cmp => true,
Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false,
}
}
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 @@ -217,6 +217,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);
CVoid, sym::c_void, c_void, Target::Enum, GenericRequirement::None;
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 @@ -98,6 +98,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 @@ -325,6 +326,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)], Ty::new_tup(tcx, &[param(0), tcx.types.bool]))
}
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 @@ -1429,6 +1429,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
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,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 @@ -300,7 +305,8 @@ impl BinOp {
BinOp::Gt => hir::BinOpKind::Gt,
BinOp::Le => hir::BinOpKind::Le,
BinOp::Ge => hir::BinOpKind::Ge,
BinOp::AddUnchecked
BinOp::Cmp
| BinOp::AddUnchecked
| BinOp::SubUnchecked
| BinOp::MulUnchecked
| BinOp::ShlUnchecked
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 @@ -784,6 +784,13 @@ impl<'tcx> TyCtxt<'tcx> {
self.get_lang_items(())
}

/// 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()
}

/// Obtain the given diagnostic item's `DefId`. Use `is_diagnostic_item` if you just want to
/// compare against another `DefId`, since `is_diagnostic_item` is cheaper.
pub fn get_diagnostic_item(self, name: Symbol) -> Option<DefId> {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/src/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
sym::wrapping_add
| sym::wrapping_sub
| sym::wrapping_mul
| sym::three_way_compare
| sym::unchecked_add
| sym::unchecked_sub
| sym::unchecked_mul
Expand All @@ -100,6 +101,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,
sym::unchecked_add => BinOp::AddUnchecked,
sym::unchecked_sub => BinOp::SubUnchecked,
sym::unchecked_mul => BinOp::MulUnchecked,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_smir/src/rustc_smir/convert/mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ impl<'tcx> Stable<'tcx> for mir::BinOp {
BinOp::Ne => stable_mir::mir::BinOp::Ne,
BinOp::Ge => stable_mir::mir::BinOp::Ge,
BinOp::Gt => stable_mir::mir::BinOp::Gt,
BinOp::Cmp => stable_mir::mir::BinOp::Cmp,
BinOp::Offset => stable_mir::mir::BinOp::Offset,
}
}
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 @@ -1658,6 +1658,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 @@ -82,7 +82,7 @@ fn check_binop(op: mir::BinOp) -> bool {
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Shl | ShlUnchecked | Shr | ShrUnchecked | Eq | Lt | Le | Ne | Ge
| Gt => true,
| Gt | Cmp => true,
Offset => false,
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/stable_mir/src/mir/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ pub enum BinOp {
Ne,
Ge,
Gt,
Cmp,
Offset,
}

Expand Down
8 changes: 8 additions & 0 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ pub struct AssertParamIsEq<T: Eq + ?Sized> {
/// ```
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
#[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 @@ -1513,12 +1514,19 @@ mod impls {
impl Ord for $t {
#[inline]
fn cmp(&self, other: &$t) -> Ordering {
#[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)
}
}
}
)*)
}
Expand Down
Loading

0 comments on commit 8811efa

Please sign in to comment.