[libcxx-commits] [libcxx] [libc++] [sort] Refactor `__cond_swap` and `__partially_sorted_swap` … (PR #86010)

[Amirreza Ashouri via libcxx-commits]

https://github.com/AMP999 created https://github.com/llvm/llvm-project/pull/86010

…for stability

The current definition of `__r` specifically denotes "true only if a swap is necessary," contrasting with the previous code's interpretation of "true when a swap is necessary, false when a swap would be incorrect, and possibly true or false when the elements are equivalent."

The same is done for `__partially_sorted_swap`. This also makes the x86-64 codegen for `__sort3` one `mov` instruction shorter.

>From 4692c22a6b4ff5bb6924b375701398e4ca9e107a Mon Sep 17 00:00:00 2001
From: Amirreza Ashouri <ar.ashouri999 at gmail.com>
Date: Wed, 20 Mar 2024 22:21:49 +0330
Subject: [PATCH] [libc++] [sort] Refactor `__cond_swap` and
 `__partially_sorted_swap` for stability

The current definition of `__r` specifically denotes "true only if a swap is necessary,"
contrasting with the previous code's interpretation of "true when a swap is necessary,
false when a swap would be incorrect, and possibly true or false when the elements
are equivalent."

The same is done for `__partially_sorted_swap`. This also makes the x86-64 codegen
for `__sort3` one `mov` instruction shorter.
---
 libcxx/include/__algorithm/sort.h | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/libcxx/include/__algorithm/sort.h b/libcxx/include/__algorithm/sort.h
index 8a5e0211cdf4c1..1cfebaaf6cce0d 100644
--- a/libcxx/include/__algorithm/sort.h
+++ b/libcxx/include/__algorithm/sort.h
@@ -161,9 +161,9 @@ template <class _Compare, class _RandomAccessIterator>
 inline _LIBCPP_HIDE_FROM_ABI void __cond_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _Compare __c) {
   // Note: this function behaves correctly even with proxy iterators (because it relies on `value_type`).
   using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
-  bool __r         = __c(*__x, *__y);
-  value_type __tmp = __r ? *__x : *__y;
-  *__y             = __r ? *__y : *__x;
+  bool __r         = __c(*__y, *__x);
+  value_type __tmp = __r ? *__y : *__x;
+  *__y             = __r ? *__x : *__y;
   *__x             = __tmp;
 }
 
@@ -174,12 +174,19 @@ inline _LIBCPP_HIDE_FROM_ABI void
 __partially_sorted_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _RandomAccessIterator __z, _Compare __c) {
   // Note: this function behaves correctly even with proxy iterators (because it relies on `value_type`).
   using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
-  bool __r         = __c(*__z, *__x);
-  value_type __tmp = __r ? *__z : *__x;
-  *__z             = __r ? *__x : *__z;
-  __r              = __c(__tmp, *__y);
-  *__x             = __r ? *__x : *__y;
-  *__y             = __r ? *__y : __tmp;
+  value_type __t      = *__x;
+  value_type __u      = *__y;
+  value_type __v      = *__z;
+  value_type __orig_t = __t;
+  bool __r            = __c(__u, __t);
+  __t                 = __r ? __u : __t;
+  __u                 = __r ? __orig_t : __u;
+  __r                 = __c(__v, __orig_t);
+  __u                 = __r ? __v : __u;
+  __v                 = __r ? __orig_t : __v;
+  *__x                = __t;
+  *__y                = __u;
+  *__z                = __v;
 }
 
 template <class,