Auto merge of #108157 - scottmcm:tuple-gt-via-partialcmp, r=dtolnay

Use `partial_cmp` to implement tuple `lt`/`le`/`ge`/`gt`

In today's implementation, `(A, B)::gt` contains calls to *both* `A::eq` *and* `A::gt`.

That's fine for primitives, but for things like `String`s it's kinda weird -- `(String, usize)::gt` has a call to both `bcmp` and `memcmp` (<https://rust.godbolt.org/z/7jbbPMesf>) because when `bcmp` says the `String`s aren't equal, it turns around and calls `memcmp` to find out which one's bigger.

This PR changes the implementation to instead implement `(A, …, C, Z)::gt` using `A::partial_cmp`, `…::partial_cmp`, `C::partial_cmp`, and `Z::gt`.  (And analogously for `lt`, `le`, and `ge`.)  That way expensive comparisons don't need to be repeated.

Technically this is an observable change on stable, so I've marked it `needs-fcp` + `T-libs-api` and will
r? rust-lang/libs-api

I'm hoping that this will be non-controversial, however, since it's very similar to the observable changes that were made to the derives (#81384 #98655) -- like those, this only changes behaviour if a type overrode behaviour in a way inconsistent with the rules for the various traits involved.

(The first commit here is #108156, adding the codegen test, which I used to make sure this doesn't regress behaviour for primitives.)

Zulip conversation about this change: <https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/.60.3E.60.20on.20Tuples/near/328392927>.

1 files changed, 121 insertions, 0 deletions

diff --git a/tests/codegen/comparison-operators-2-tuple.rs b/tests/codegen/comparison-operators-2-tuple.rs
new file mode 100644
index 00000000000..a9d25e3b53c
--- /dev/null
+++ b/tests/codegen/comparison-operators-2-tuple.rs
@@ -0,0 +1,121 @@
+// compile-flags: -C opt-level=1 -Z merge-functions=disabled
+// min-llvm-version: 15.0
+// only-x86_64
+
+#![crate_type = "lib"]
+
+use std::cmp::Ordering;
+
+type TwoTuple = (i16, u16);
+
+//
+// The operators are all overridden directly, so should optimize easily.
+//
+// Yes, the `s[lg]t` is correct for the `[lg]e` version because it's only used
+// in the side of the select where we know the values are *not* equal.
+//
+
+// CHECK-LABEL: @check_lt_direct
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_lt_direct(a: TwoTuple, b: TwoTuple) -> bool {
+    // CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP0:.+]] = icmp slt i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP1:.+]] = icmp ult i16 %[[A1]], %[[B1]]
+    // CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // CHECK: ret i1 %[[R]]
+    a < b
+}
+
+// CHECK-LABEL: @check_le_direct
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_le_direct(a: TwoTuple, b: TwoTuple) -> bool {
+    // CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP0:.+]] = icmp slt i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP1:.+]] = icmp ule i16 %[[A1]], %[[B1]]
+    // CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // CHECK: ret i1 %[[R]]
+    a <= b
+}
+
+// CHECK-LABEL: @check_gt_direct
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_gt_direct(a: TwoTuple, b: TwoTuple) -> bool {
+    // CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP0:.+]] = icmp sgt i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP1:.+]] = icmp ugt i16 %[[A1]], %[[B1]]
+    // CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // CHECK: ret i1 %[[R]]
+    a > b
+}
+
+// CHECK-LABEL: @check_ge_direct
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_ge_direct(a: TwoTuple, b: TwoTuple) -> bool {
+    // CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP0:.+]] = icmp sgt i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP1:.+]] = icmp uge i16 %[[A1]], %[[B1]]
+    // CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // CHECK: ret i1 %[[R]]
+    a >= b
+}
+
+//
+// These ones are harder, since there are more intermediate values to remove.
+//
+// `<` seems to be getting lucky right now, so test that doesn't regress.
+//
+// The others, however, aren't managing to optimize away the extra `select`s yet.
+// See <https://github.com/rust-lang/rust/issues/106107> for more about this.
+//
+
+// CHECK-LABEL: @check_lt_via_cmp
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_lt_via_cmp(a: TwoTuple, b: TwoTuple) -> bool {
+    // CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP0:.+]] = icmp slt i16 %[[A0]], %[[B0]]
+    // CHECK-DAG: %[[CMP1:.+]] = icmp ult i16 %[[A1]], %[[B1]]
+    // CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // CHECK: ret i1 %[[R]]
+    Ord::cmp(&a, &b).is_lt()
+}
+
+// CHECK-LABEL: @check_le_via_cmp
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_le_via_cmp(a: TwoTuple, b: TwoTuple) -> bool {
+    // FIXME-CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP0:.+]] = icmp sle i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP1:.+]] = icmp ule i16 %[[A1]], %[[B1]]
+    // FIXME-CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // FIXME-CHECK: ret i1 %[[R]]
+    Ord::cmp(&a, &b).is_le()
+}
+
+// CHECK-LABEL: @check_gt_via_cmp
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_gt_via_cmp(a: TwoTuple, b: TwoTuple) -> bool {
+    // FIXME-CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP0:.+]] = icmp sgt i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP1:.+]] = icmp ugt i16 %[[A1]], %[[B1]]
+    // FIXME-CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // FIXME-CHECK: ret i1 %[[R]]
+    Ord::cmp(&a, &b).is_gt()
+}
+
+// CHECK-LABEL: @check_ge_via_cmp
+// CHECK-SAME: (i16 noundef %[[A0:.+]], i16 noundef %[[A1:.+]], i16 noundef %[[B0:.+]], i16 noundef %[[B1:.+]])
+#[no_mangle]
+pub fn check_ge_via_cmp(a: TwoTuple, b: TwoTuple) -> bool {
+    // FIXME-CHECK-DAG: %[[EQ:.+]] = icmp eq i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP0:.+]] = icmp sge i16 %[[A0]], %[[B0]]
+    // FIXME-CHECK-DAG: %[[CMP1:.+]] = icmp uge i16 %[[A1]], %[[B1]]
+    // FIXME-CHECK: %[[R:.+]] = select i1 %[[EQ]], i1 %[[CMP1]], i1 %[[CMP0]]
+    // FIXME-CHECK: ret i1 %[[R]]
+    Ord::cmp(&a, &b).is_ge()
+}