Improve what we generate in debug mode

Author: scottmcm

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 <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)
+                if bx.cx().tcx().sess.opts.optimize == OptLevel::No {
+                    // FIXME: This actually generates tighter assembly, and is a classic trick
+                    // <https://graphics.stanford.edu/~seander/bithacks.html#CopyIntegerSign>
+                    // 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 <https://github.com/llvm/llvm-project/issues/73417>), 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 <https://github.com/rust-lang/rust/pull/63767>.
+                    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)
+                }
             }
         }
     }

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)
+}

tests/codegen/integer-cmp-raw.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)
+}