Skip to content

Preserve signed zeros in VQSort sorting networks - #3544

Merged
copybara-service[bot] merged 2 commits into
google:masterfrom
LittlePyx:fix/vqsort-network-signed-zeros
Nov 11, 2026
Merged

copybara-service[bot] merged 2 commits into
google:masterfrom
LittlePyx:fix/vqsort-network-signed-zeros

Conversation

@LittlePyx

Copy link
Copy Markdown
Contributor

Sorting [+0.0, -0.0] can produce [-0.0, -0.0] in VQSort's short-array sorting network. Both outputs compare equal, so a sortedness check misses the lost positive zero. This fixes the sorting-network case reported in #3543, which addressed the two-value partition.

On x86, floating-point min and max both select their second operand when the inputs are opposite zeros. An ascending compare-exchange between registers therefore needs min(a, b) and max(b, a) to keep one copy of each input. A mirrored comparison reads a vector and a permuted copy of itself. The lane supplying the minimum sees (a, b), and its partner supplying the maximum already sees (b, a). Reversing the max operands again would duplicate one zero. The patch marks these mirrored calls explicitly and bypasses the 64-bit compare-and-blend shortcut where merging mirrored floating-point lanes would lose a sign.

The regression test places a single negative zero at each position in a two-element input and in a full sorting network, then checks its count. It fails on the original implementation. The patched sort unit tests passed on native AVX2 and under Intel SDE emulating Skylake-X. In the 128-double reproducer, ascending and descending sorts now both preserve all 40 negative zeros, up from 2 and 20 respectively. AVX-512 validation was simulation only.

On a Ryzen 7 5800H with GCC 13.3, AVX2 VQSortStatic took 2.06–6.23% longer (0.18–0.89 ns per sort) for two- and three-element float32 arrays. These are paired median differences across three runs, both sort directions, and random and mixed-signed-zero inputs, with input copies outside the timer. At 128 float32 elements, median time ranged from 2.53% faster to 3.10% slower.

Codex assisted with writing and testing the patch and drafting this PR description.

@jan-wassenberg jan-wassenberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. One question: I was not familiar with this ordering behavior of the min/max instructions relating to +-0. Is this x86 only or do other ISAs also have this?

Comment thread hwy/contrib/sort/traits-inl.h Outdated
return IfVecThenElse(DupOdd(cmp), swapped, v);
#else
Sort2(d, v, swapped);
Sort2<true>(d, v, swapped);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer self-documenting enum or tag struct types over bool.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 7aa2787. The mirrored calls now read Sort2<PairLayout::kMirrored>, with kIndependent as the default. The sort unit tests passed on native AVX2 and under Skylake-X emulation.

@LittlePyx

Copy link
Copy Markdown
Contributor Author

Thanks for asking. WebAssembly SIMD is another example where operand order matters. Its pmin and pmax both keep the first operand on a tie. Highway passes (b, a) to those instructions, so at the Highway API level both Min(+0, -0) and Max(+0, -0) return -0, just as on x86. Swapping the inputs gives +0 for both. Wasm SIMD semantics · Highway's Min · Highway's Max

I checked the f32 and f64 Wasm instructions in Node with both operand orders. For comparison, a Highway NEON probe under QEMU (AArch64 Cortex-A76) gave Min = -0 and Max = +0 in both orders. Reversing the operands of the second result preserves both zeros in either case.

@jan-wassenberg jan-wassenberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating.

@copybara-service
copybara-service Bot merged commit 7101cd0 into google:master Nov 11, 2026
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants