From 9681c9a352ca6f9d9d3c9f62285f99fff1380fc1 Mon Sep 17 00:00:00 2001 From: LittlePyx <111182516+LittlePyx@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:12:41 +0800 Subject: [PATCH 1/2] Preserve signed zeros in VQSort sorting networks --- hwy/contrib/sort/sort_unit_test.cc | 25 ++++++++++++++++++++++++ hwy/contrib/sort/sorting_networks-inl.h | 6 +++--- hwy/contrib/sort/traits-inl.h | 26 ++++++++++++++++--------- hwy/contrib/sort/traits128-inl.h | 2 +- 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/hwy/contrib/sort/sort_unit_test.cc b/hwy/contrib/sort/sort_unit_test.cc index e0ba54e5ab..43afc3882a 100644 --- a/hwy/contrib/sort/sort_unit_test.cc +++ b/hwy/contrib/sort/sort_unit_test.cc @@ -309,6 +309,27 @@ static HWY_NOINLINE void TestBaseCase() { TestBaseCase01(); } +template +static HWY_NOINLINE void TestBaseCaseSignedZero() { + using T = typename Traits::LaneType; + const SortTag d; + const size_t num = SortConstants::BaseCaseNumLanes(Lanes(d)); + auto keys = hwy::AllocateAligned(num); + auto buf = hwy::AllocateAligned(SortConstants::BufNum(Lanes(d))); + HWY_ASSERT(keys && buf); + // Two keys exercise one compare-exchange. The full network also compares + // lanes within the same vector; place the negative zero on either side. + for (size_t len : {size_t{2}, num}) { + for (size_t pos = 0; pos < len; ++pos) { + for (size_t i = 0; i < len; ++i) keys[i] = i == pos ? -T{0} : T{0}; + detail::BaseCase(d, SharedTraits(), keys.get(), len, buf.get()); + size_t count = 0; + for (size_t i = 0; i < len; ++i) count += ScalarSignBit(keys[i]); + HWY_ASSERT_EQ(size_t{1}, count); + } + } +} + HWY_NOINLINE void TestAllBaseCase() { // Workaround for stack overflow on MSVC debug. #if defined(_MSC_VER) @@ -317,6 +338,10 @@ HWY_NOINLINE void TestAllBaseCase() { TestBaseCase > >(); TestBaseCase > >(); + TestBaseCaseSignedZero>>(); +#if HWY_HAVE_FLOAT64 + TestBaseCaseSignedZero>>(); +#endif #if !HAVE_INTEL TestBaseCase >(); TestBaseCase >(); diff --git a/hwy/contrib/sort/sorting_networks-inl.h b/hwy/contrib/sort/sorting_networks-inl.h index 54b1523073..5e1c327f59 100644 --- a/hwy/contrib/sort/sorting_networks-inl.h +++ b/hwy/contrib/sort/sorting_networks-inl.h @@ -51,7 +51,7 @@ struct SharedTraits : public Base { HWY_INLINE Vec SortPairsDistance2(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->SwapAdjacentPairs(d, v); - base->Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return base->OddEvenPairs(d, swapped, v); } @@ -60,7 +60,7 @@ struct SharedTraits : public Base { HWY_INLINE Vec SortPairsReverse8(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys8(d, v); - base->Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return base->OddEvenQuads(d, swapped, v); } @@ -70,7 +70,7 @@ struct SharedTraits : public Base { const Base* base = static_cast(this); static_assert(Constants::kMaxCols <= 16, "Need actual Reverse16"); Vec swapped = base->ReverseKeys(d, v); - base->Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return ConcatUpperLower(d, swapped, v); // 8 = half of the vector } }; diff --git a/hwy/contrib/sort/traits-inl.h b/hwy/contrib/sort/traits-inl.h index b02ed8b9ab..a547e674ba 100644 --- a/hwy/contrib/sort/traits-inl.h +++ b/hwy/contrib/sort/traits-inl.h @@ -550,15 +550,16 @@ struct TraitsLane : public Base { // according to Base. // Corresponds to a conditional swap, which is one "node" of a sorting // network. Min/Max are cheaper than compare + blend at least for integers. - template + template HWY_INLINE void Sort2(D d, Vec& a, Vec& b) const { const Base* base = static_cast(this); const Vec a_copy = a; - // Prior to AVX3, there is no native 64-bit Min/Max, so they compile to 4 - // instructions. We can reduce it to a compare + 2 IfThenElse. + // Prior to AVX3 there is no native 64-bit integer Min/Max. Use a compare + // and two blends, except for mirrored floats: selecting opposite inputs + // on a tie there would duplicate one zero after the lanes are merged. #if HWY_AVX3 < HWY_TARGET && HWY_TARGET <= HWY_SSSE3 - if (sizeof(TFromD) == 8) { + if (sizeof(TFromD) == 8 && !(kMirror && IsFloat>())) { const Mask cmp = base->Compare(d, a, b); a = IfThenElse(cmp, a, b); b = IfThenElse(cmp, b, a_copy); @@ -566,7 +567,14 @@ struct TraitsLane : public Base { } #endif a = base->First(d, a, b); - b = base->Last(d, a_copy, b); + // Some floating-point Min/Max instructions both choose the second operand + // for equal zeros. Keep one of each input. Mirrored lanes already compare + // the same pair in opposite orders before selecting one result per lane. + if constexpr (!kMirror && IsFloat>()) { + b = base->Last(d, b, a_copy); + } else { + b = base->Last(d, a_copy, b); + } } // Conditionally swaps even-numbered lanes with their odd-numbered neighbor. @@ -580,7 +588,7 @@ struct TraitsLane : public Base { const Vec cmp = VecFromMask(d, base->Compare(d, v, swapped)); return IfVecThenElse(DupOdd(cmp), swapped, v); #else - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenKeys(swapped, v); #endif } @@ -590,7 +598,7 @@ struct TraitsLane : public Base { HWY_INLINE Vec SortPairsDistance1(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys2(d, v); - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenKeys(swapped, v); } @@ -599,7 +607,7 @@ struct TraitsLane : public Base { HWY_INLINE Vec SortPairsReverse4(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys4(d, v); - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenPairs(d, swapped, v); } @@ -610,7 +618,7 @@ struct TraitsLane : public Base { Vec swapped = base->SwapAdjacentQuads(d, v); // Only used in Merge16, so this will not be used on AVX2 (which only has 4 // u64 lanes), so skip the above optimization for 64-bit AVX2. - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenQuads(d, swapped, v); } }; diff --git a/hwy/contrib/sort/traits128-inl.h b/hwy/contrib/sort/traits128-inl.h index 4a1cc223de..6f8d6e7ce3 100644 --- a/hwy/contrib/sort/traits128-inl.h +++ b/hwy/contrib/sort/traits128-inl.h @@ -494,7 +494,7 @@ struct Traits128 : public Base { return v; } - template + template HWY_INLINE void Sort2(D d, Vec& a, Vec& b) const { const Base* base = static_cast(this); From 7aa2787ecea0f95efa5048eee9c93149d72faa6d Mon Sep 17 00:00:00 2001 From: LittlePyx <111182516+LittlePyx@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:53:25 +0800 Subject: [PATCH 2/2] Name the pair layouts used by VQSort compare-exchanges --- hwy/contrib/sort/shared-inl.h | 4 ++++ hwy/contrib/sort/sorting_networks-inl.h | 6 +++--- hwy/contrib/sort/traits-inl.h | 15 ++++++++------- hwy/contrib/sort/traits128-inl.h | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/hwy/contrib/sort/shared-inl.h b/hwy/contrib/sort/shared-inl.h index 0f35892ffc..5ebe3b9897 100644 --- a/hwy/contrib/sort/shared-inl.h +++ b/hwy/contrib/sort/shared-inl.h @@ -165,6 +165,10 @@ static_assert(SortConstants::MaxBufBytes<2>(64) <= 1664, "Unexpectedly high"); namespace hwy { namespace HWY_NAMESPACE { +namespace detail { +enum class PairLayout { kIndependent, kMirrored }; +} // namespace detail + // Default tag / vector width selector. #if HWY_TARGET == HWY_RVV // Use LMUL = 1/2; for SEW=64 this ends up emulated via VSETVLI. diff --git a/hwy/contrib/sort/sorting_networks-inl.h b/hwy/contrib/sort/sorting_networks-inl.h index 5e1c327f59..92c2dfad48 100644 --- a/hwy/contrib/sort/sorting_networks-inl.h +++ b/hwy/contrib/sort/sorting_networks-inl.h @@ -51,7 +51,7 @@ struct SharedTraits : public Base { HWY_INLINE Vec SortPairsDistance2(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->SwapAdjacentPairs(d, v); - base->template Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return base->OddEvenPairs(d, swapped, v); } @@ -60,7 +60,7 @@ struct SharedTraits : public Base { HWY_INLINE Vec SortPairsReverse8(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys8(d, v); - base->template Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return base->OddEvenQuads(d, swapped, v); } @@ -70,7 +70,7 @@ struct SharedTraits : public Base { const Base* base = static_cast(this); static_assert(Constants::kMaxCols <= 16, "Need actual Reverse16"); Vec swapped = base->ReverseKeys(d, v); - base->template Sort2(d, v, swapped); + base->template Sort2(d, v, swapped); return ConcatUpperLower(d, swapped, v); // 8 = half of the vector } }; diff --git a/hwy/contrib/sort/traits-inl.h b/hwy/contrib/sort/traits-inl.h index a547e674ba..3a4d8506e6 100644 --- a/hwy/contrib/sort/traits-inl.h +++ b/hwy/contrib/sort/traits-inl.h @@ -550,7 +550,7 @@ struct TraitsLane : public Base { // according to Base. // Corresponds to a conditional swap, which is one "node" of a sorting // network. Min/Max are cheaper than compare + blend at least for integers. - template + template HWY_INLINE void Sort2(D d, Vec& a, Vec& b) const { const Base* base = static_cast(this); @@ -559,7 +559,8 @@ struct TraitsLane : public Base { // and two blends, except for mirrored floats: selecting opposite inputs // on a tie there would duplicate one zero after the lanes are merged. #if HWY_AVX3 < HWY_TARGET && HWY_TARGET <= HWY_SSSE3 - if (sizeof(TFromD) == 8 && !(kMirror && IsFloat>())) { + if (sizeof(TFromD) == 8 && + !(kLayout == PairLayout::kMirrored && IsFloat>())) { const Mask cmp = base->Compare(d, a, b); a = IfThenElse(cmp, a, b); b = IfThenElse(cmp, b, a_copy); @@ -570,7 +571,7 @@ struct TraitsLane : public Base { // Some floating-point Min/Max instructions both choose the second operand // for equal zeros. Keep one of each input. Mirrored lanes already compare // the same pair in opposite orders before selecting one result per lane. - if constexpr (!kMirror && IsFloat>()) { + if constexpr (kLayout == PairLayout::kIndependent && IsFloat>()) { b = base->Last(d, b, a_copy); } else { b = base->Last(d, a_copy, b); @@ -588,7 +589,7 @@ struct TraitsLane : public Base { const Vec cmp = VecFromMask(d, base->Compare(d, v, swapped)); return IfVecThenElse(DupOdd(cmp), swapped, v); #else - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenKeys(swapped, v); #endif } @@ -598,7 +599,7 @@ struct TraitsLane : public Base { HWY_INLINE Vec SortPairsDistance1(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys2(d, v); - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenKeys(swapped, v); } @@ -607,7 +608,7 @@ struct TraitsLane : public Base { HWY_INLINE Vec SortPairsReverse4(D d, Vec v) const { const Base* base = static_cast(this); Vec swapped = base->ReverseKeys4(d, v); - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenPairs(d, swapped, v); } @@ -618,7 +619,7 @@ struct TraitsLane : public Base { Vec swapped = base->SwapAdjacentQuads(d, v); // Only used in Merge16, so this will not be used on AVX2 (which only has 4 // u64 lanes), so skip the above optimization for 64-bit AVX2. - Sort2(d, v, swapped); + Sort2(d, v, swapped); return base->OddEvenQuads(d, swapped, v); } }; diff --git a/hwy/contrib/sort/traits128-inl.h b/hwy/contrib/sort/traits128-inl.h index 6f8d6e7ce3..8fcbb7497b 100644 --- a/hwy/contrib/sort/traits128-inl.h +++ b/hwy/contrib/sort/traits128-inl.h @@ -494,7 +494,7 @@ struct Traits128 : public Base { return v; } - template + template HWY_INLINE void Sort2(D d, Vec& a, Vec& b) const { const Base* base = static_cast(this);