From 82551cac88e4be906feea83d7beb7077e4cf1a5e Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Mon, 27 Nov 2023 00:28:42 -0800 Subject: [PATCH] Improve what we generate in debug mode --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 41 +++++++++------ tests/assembly/x86_64-cmp.rs | 51 +++++++++++++++++++ tests/codegen/integer-cmp-raw.rs | 30 ----------- tests/codegen/intrinsics/three_way_compare.rs | 47 +++++++++++++++++ 4 files changed, 123 insertions(+), 46 deletions(-) create mode 100644 tests/assembly/x86_64-cmp.rs delete mode 100644 tests/codegen/integer-cmp-raw.rs create mode 100644 tests/codegen/intrinsics/three_way_compare.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 7071091befb6b..c237564a025f3 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -885,23 +885,32 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { 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 , - // 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 - // ) or once we - // update to new LLVM, something to take advantage of the new folds in - // . 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) + if bx.cx().tcx().sess.opts.optimize == OptLevel::No { + // FIXME: This actually generates tighter assembly, and is a classic trick + // + // However, as of 2023-11 it optimizes worse in things like derived + // `PartialOrd`, so only use it in debug for now. Once LLVM can handle it + // better (see ), it'll + // be worth trying it in optimized builds as well. + let is_gt = bx.icmp(pred(hir::BinOpKind::Gt), lhs, rhs); + let gtext = bx.zext(is_gt, bx.type_i8()); + let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs); + let ltext = bx.zext(is_lt, bx.type_i8()); + bx.unchecked_ssub(gtext, ltext) + } else { + // These operations are those expected by `tests/codegen/integer-cmp.rs`, + // from . + 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) + } } } } diff --git a/tests/assembly/x86_64-cmp.rs b/tests/assembly/x86_64-cmp.rs new file mode 100644 index 0000000000000..f50b70c9d22ce --- /dev/null +++ b/tests/assembly/x86_64-cmp.rs @@ -0,0 +1,51 @@ +// revisions: DEBUG OPTIM +// [DEBUG] compile-flags: -C opt-level=0 +// [OPTIM] compile-flags: -C opt-level=3 +// assembly-output: emit-asm +// compile-flags: --crate-type=lib -C llvm-args=-x86-asm-syntax=intel +// only-x86_64 +// ignore-sgx + +#![feature(core_intrinsics)] + +use std::intrinsics::three_way_compare; + +#[no_mangle] +// CHECK-LABEL: signed_cmp: +pub fn signed_cmp(a: i16, b: i16) -> std::cmp::Ordering { + // DEBUG: cmp + // DEBUG: setg + // DEBUG: and + // DEBUG: cmp + // DEBUG: setl + // DEBUG: and + // DEBUG: sub + + // OPTIM: xor + // OPTIM: cmp + // OPTIM: setne + // OPTIM: mov + // OPTIM: cmovge + // OPTIM: ret + three_way_compare(a, b) +} + +#[no_mangle] +// CHECK-LABEL: unsigned_cmp: +pub fn unsigned_cmp(a: u16, b: u16) -> std::cmp::Ordering { + // DEBUG: cmp + // DEBUG: seta + // DEBUG: and + // DEBUG: cmp + // DEBUG: setb + // DEBUG: and + // DEBUG: sub + + // OPTIM: xor + // OPTIM: cmp + // OPTIM: setne + // OPTIM: mov + // OPTIM: cmovae + // OPTIM: ret + three_way_compare(a, b) +} diff --git a/tests/codegen/integer-cmp-raw.rs b/tests/codegen/integer-cmp-raw.rs deleted file mode 100644 index 00fa037e5a07e..0000000000000 --- a/tests/codegen/integer-cmp-raw.rs +++ /dev/null @@ -1,30 +0,0 @@ -// This is test for more optimal Ord implementation for integers. -// See for more info. - -// compile-flags: -C opt-level=3 -C no-prepopulate-passes - -#![crate_type = "lib"] - -use std::cmp::Ordering; - -// CHECK-LABEL: @cmp_signed -#[no_mangle] -pub fn cmp_signed(a: i64, b: i64) -> Ordering { -// CHECK: %[[LT:.+]] = icmp slt i64 -// CHECK: %[[NE:.+]] = icmp ne i64 -// CHECK: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0 -// CHECK: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]] -// CHECK: ret i8 %[[CGEL]] - a.cmp(&b) -} - -// CHECK-LABEL: @cmp_unsigned -#[no_mangle] -pub fn cmp_unsigned(a: u32, b: u32) -> Ordering { -// CHECK: %[[LT:.+]] = icmp ult i32 -// CHECK: %[[NE:.+]] = icmp ne i32 -// CHECK: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0 -// CHECK: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]] -// CHECK: ret i8 %[[CGEL]] - a.cmp(&b) -} diff --git a/tests/codegen/intrinsics/three_way_compare.rs b/tests/codegen/intrinsics/three_way_compare.rs new file mode 100644 index 0000000000000..0c56c390b478a --- /dev/null +++ b/tests/codegen/intrinsics/three_way_compare.rs @@ -0,0 +1,47 @@ +// revisions: DEBUG OPTIM +// [DEBUG] compile-flags: -C opt-level=0 +// [OPTIM] compile-flags: -C opt-level=3 +// compile-flags: -C no-prepopulate-passes + +#![crate_type = "lib"] +#![feature(core_intrinsics)] + +use std::intrinsics::three_way_compare; + +#[no_mangle] +// CHECK-LABEL: @signed_cmp +// DEBUG-SAME: (i16 %a, i16 %b) +// OPTIM-SAME: (i16 noundef %a, i16 noundef %b) +pub fn signed_cmp(a: i16, b: i16) -> std::cmp::Ordering { + // DEBUG: %[[GT:.+]] = icmp sgt i16 %a, %b + // DEBUG: %[[ZGT:.+]] = zext i1 %[[GT]] to i8 + // DEBUG: %[[LT:.+]] = icmp slt i16 %a, %b + // DEBUG: %[[ZLT:.+]] = zext i1 %[[LT]] to i8 + // DEBUG: %[[R:.+]] = sub nsw i8 %[[ZGT]], %[[ZLT]] + + // OPTIM: %[[LT:.+]] = icmp slt i16 %a, %b + // OPTIM: %[[NE:.+]] = icmp ne i16 %a, %b + // OPTIM: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0 + // OPTIM: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]] + // OPTIM: ret i8 %[[CGEL]] + three_way_compare(a, b) +} + +#[no_mangle] +// CHECK-LABEL: @unsigned_cmp +// DEBUG-SAME: (i16 %a, i16 %b) +// OPTIM-SAME: (i16 noundef %a, i16 noundef %b) +pub fn unsigned_cmp(a: u16, b: u16) -> std::cmp::Ordering { + // DEBUG: %[[GT:.+]] = icmp ugt i16 %a, %b + // DEBUG: %[[ZGT:.+]] = zext i1 %[[GT]] to i8 + // DEBUG: %[[LT:.+]] = icmp ult i16 %a, %b + // DEBUG: %[[ZLT:.+]] = zext i1 %[[LT]] to i8 + // DEBUG: %[[R:.+]] = sub nsw i8 %[[ZGT]], %[[ZLT]] + + // OPTIM: %[[LT:.+]] = icmp ult i16 %a, %b + // OPTIM: %[[NE:.+]] = icmp ne i16 %a, %b + // OPTIM: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0 + // OPTIM: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]] + // OPTIM: ret i8 %[[CGEL]] + three_way_compare(a, b) +}