Skip to content

Commit 82551ca

Browse files
committed
Improve what we generate in debug mode
1 parent 9cfc79a commit 82551ca

File tree

4 files changed

+123
-46
lines changed

4 files changed

+123
-46
lines changed

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+25-16
Original file line numberDiff line numberDiff line change
@@ -885,23 +885,32 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
885885
mir::BinOp::Cmp => {
886886
use std::cmp::Ordering;
887887
debug_assert!(!is_float);
888-
// FIXME: To avoid this PR changing behaviour, the operations used
889-
// here are those from <https://github.com/rust-lang/rust/pull/63767>,
890-
// as tested by `tests/codegen/integer-cmp.rs`.
891-
// Something in future might want to pick different ones. For example,
892-
// maybe the ones from Clang's `<=>` operator in C++20 (see
893-
// <https://github.com/llvm/llvm-project/issues/60012>) or once we
894-
// update to new LLVM, something to take advantage of the new folds in
895-
// <https://github.com/llvm/llvm-project/issues/59666>.
896888
let pred = |op| base::bin_op_to_icmp_predicate(op, is_signed);
897-
let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs);
898-
let is_ne = bx.icmp(pred(hir::BinOpKind::Ne), lhs, rhs);
899-
let ge = bx.select(
900-
is_ne,
901-
bx.cx().const_i8(Ordering::Greater as i8),
902-
bx.cx().const_i8(Ordering::Equal as i8),
903-
);
904-
bx.select(is_lt, bx.cx().const_i8(Ordering::Less as i8), ge)
889+
if bx.cx().tcx().sess.opts.optimize == OptLevel::No {
890+
// FIXME: This actually generates tighter assembly, and is a classic trick
891+
// <https://graphics.stanford.edu/~seander/bithacks.html#CopyIntegerSign>
892+
// However, as of 2023-11 it optimizes worse in things like derived
893+
// `PartialOrd`, so only use it in debug for now. Once LLVM can handle it
894+
// better (see <https://github.com/llvm/llvm-project/issues/73417>), it'll
895+
// be worth trying it in optimized builds as well.
896+
let is_gt = bx.icmp(pred(hir::BinOpKind::Gt), lhs, rhs);
897+
let gtext = bx.zext(is_gt, bx.type_i8());
898+
let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs);
899+
let ltext = bx.zext(is_lt, bx.type_i8());
900+
bx.unchecked_ssub(gtext, ltext)
901+
} else {
902+
// These operations are those expected by `tests/codegen/integer-cmp.rs`,
903+
// from <https://github.com/rust-lang/rust/pull/63767>.
904+
let pred = |op| base::bin_op_to_icmp_predicate(op, is_signed);
905+
let is_lt = bx.icmp(pred(hir::BinOpKind::Lt), lhs, rhs);
906+
let is_ne = bx.icmp(pred(hir::BinOpKind::Ne), lhs, rhs);
907+
let ge = bx.select(
908+
is_ne,
909+
bx.cx().const_i8(Ordering::Greater as i8),
910+
bx.cx().const_i8(Ordering::Equal as i8),
911+
);
912+
bx.select(is_lt, bx.cx().const_i8(Ordering::Less as i8), ge)
913+
}
905914
}
906915
}
907916
}

tests/assembly/x86_64-cmp.rs

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// revisions: DEBUG OPTIM
2+
// [DEBUG] compile-flags: -C opt-level=0
3+
// [OPTIM] compile-flags: -C opt-level=3
4+
// assembly-output: emit-asm
5+
// compile-flags: --crate-type=lib -C llvm-args=-x86-asm-syntax=intel
6+
// only-x86_64
7+
// ignore-sgx
8+
9+
#![feature(core_intrinsics)]
10+
11+
use std::intrinsics::three_way_compare;
12+
13+
#[no_mangle]
14+
// CHECK-LABEL: signed_cmp:
15+
pub fn signed_cmp(a: i16, b: i16) -> std::cmp::Ordering {
16+
// DEBUG: cmp
17+
// DEBUG: setg
18+
// DEBUG: and
19+
// DEBUG: cmp
20+
// DEBUG: setl
21+
// DEBUG: and
22+
// DEBUG: sub
23+
24+
// OPTIM: xor
25+
// OPTIM: cmp
26+
// OPTIM: setne
27+
// OPTIM: mov
28+
// OPTIM: cmovge
29+
// OPTIM: ret
30+
three_way_compare(a, b)
31+
}
32+
33+
#[no_mangle]
34+
// CHECK-LABEL: unsigned_cmp:
35+
pub fn unsigned_cmp(a: u16, b: u16) -> std::cmp::Ordering {
36+
// DEBUG: cmp
37+
// DEBUG: seta
38+
// DEBUG: and
39+
// DEBUG: cmp
40+
// DEBUG: setb
41+
// DEBUG: and
42+
// DEBUG: sub
43+
44+
// OPTIM: xor
45+
// OPTIM: cmp
46+
// OPTIM: setne
47+
// OPTIM: mov
48+
// OPTIM: cmovae
49+
// OPTIM: ret
50+
three_way_compare(a, b)
51+
}

tests/codegen/integer-cmp-raw.rs

-30
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// revisions: DEBUG OPTIM
2+
// [DEBUG] compile-flags: -C opt-level=0
3+
// [OPTIM] compile-flags: -C opt-level=3
4+
// compile-flags: -C no-prepopulate-passes
5+
6+
#![crate_type = "lib"]
7+
#![feature(core_intrinsics)]
8+
9+
use std::intrinsics::three_way_compare;
10+
11+
#[no_mangle]
12+
// CHECK-LABEL: @signed_cmp
13+
// DEBUG-SAME: (i16 %a, i16 %b)
14+
// OPTIM-SAME: (i16 noundef %a, i16 noundef %b)
15+
pub fn signed_cmp(a: i16, b: i16) -> std::cmp::Ordering {
16+
// DEBUG: %[[GT:.+]] = icmp sgt i16 %a, %b
17+
// DEBUG: %[[ZGT:.+]] = zext i1 %[[GT]] to i8
18+
// DEBUG: %[[LT:.+]] = icmp slt i16 %a, %b
19+
// DEBUG: %[[ZLT:.+]] = zext i1 %[[LT]] to i8
20+
// DEBUG: %[[R:.+]] = sub nsw i8 %[[ZGT]], %[[ZLT]]
21+
22+
// OPTIM: %[[LT:.+]] = icmp slt i16 %a, %b
23+
// OPTIM: %[[NE:.+]] = icmp ne i16 %a, %b
24+
// OPTIM: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0
25+
// OPTIM: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]]
26+
// OPTIM: ret i8 %[[CGEL]]
27+
three_way_compare(a, b)
28+
}
29+
30+
#[no_mangle]
31+
// CHECK-LABEL: @unsigned_cmp
32+
// DEBUG-SAME: (i16 %a, i16 %b)
33+
// OPTIM-SAME: (i16 noundef %a, i16 noundef %b)
34+
pub fn unsigned_cmp(a: u16, b: u16) -> std::cmp::Ordering {
35+
// DEBUG: %[[GT:.+]] = icmp ugt i16 %a, %b
36+
// DEBUG: %[[ZGT:.+]] = zext i1 %[[GT]] to i8
37+
// DEBUG: %[[LT:.+]] = icmp ult i16 %a, %b
38+
// DEBUG: %[[ZLT:.+]] = zext i1 %[[LT]] to i8
39+
// DEBUG: %[[R:.+]] = sub nsw i8 %[[ZGT]], %[[ZLT]]
40+
41+
// OPTIM: %[[LT:.+]] = icmp ult i16 %a, %b
42+
// OPTIM: %[[NE:.+]] = icmp ne i16 %a, %b
43+
// OPTIM: %[[CGE:.+]] = select i1 %[[NE]], i8 1, i8 0
44+
// OPTIM: %[[CGEL:.+]] = select i1 %[[LT]], i8 -1, i8 %[[CGE]]
45+
// OPTIM: ret i8 %[[CGEL]]
46+
three_way_compare(a, b)
47+
}

0 commit comments

Comments
 (0)