Conversation
Min over a span of float or double walks it one element at a time, while the integer overloads reach the vectorized MemoryExtensions.Min. Compare a vector at a time instead, reducing the lanes once the loop ends. The sequential walk returns the first NaN it meets, so the vectorized loop abandons the block and hands the whole span back to that walk as soon as a vector contains one, rather than trying to locate it. It also keeps the first of two equal values, which matters only for zero, since negative and positive zero compare equal while the reduction may keep either: when the result is zero, the first zero in the span is returned. Measured on arm64 with Vector128: 3.9x for float and 1.8x for double at 1K elements, 2.9x and 1.5x at a million.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
| // appears, since the first NaN is the result and the walk already reports it. | ||
| if (Vector128.IsHardwareAccelerated && Vector128<T>.IsSupported && span.Length >= Vector128<T>.Count * 2) | ||
| { | ||
| ref T first = ref MemoryMarshal.GetReference(span); |
There was a problem hiding this comment.
Same here: Rewrite to safe code
PS: Is it worth splitting this into 3 PRs ?
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
|
Switched to safe loads: the vectors now come from Correctness re-checked on the safe form: 80,000 cases, 0 value mismatches and 0 signed-zero differences. One thing you should know before deciding, because it changes the numbers in the description: the bounds check is not free at small and medium lengths. Measuring both load forms in the same process (arm64,
So the two converge once the span is large and the check is amortized, and the safe form gives up roughly a quarter of the gain at a thousand elements. (My 65,536 row came out at 2.00x safe against 3.28x unsafe, which does not fit its neighbours on either side, so I am treating that one as noise rather than signal on this machine.) The description's numbers were taken on the unsafe form and I have not edited them yet — tell me which form you want and I will make the description match the code. |
There should be no bound checks if it's written in a more idiomatic and safe way (not how this PR is written). see #127506 |
|
Also built the runtime locally since my earlier note: |
|
Folded into #134409 as you suggested, so the whole float min/max change is in one place rather than spread over three PRs. The bounds-check point is addressed there too — walking the span forward instead of re-slicing by index removes the check, and the numbers are in that PR. Closing this one. |
Minover a span offloatordoublewalks it one element at a time:The integer overloads reach the vectorized
MemoryExtensions.Min, but the floating-point ones do not, because their NaN behaviour differs from the comparer-based one.Change
Spans of at least two vectors are now compared a vector at a time, with the lanes reduced once the loop ends. Two details of the sequential behaviour are preserved deliberately:
-0.0 == +0.0, so[+0.0, -0.0]returns+0.0while[-0.0, +0.0]returns-0.0. A vector reduction can keep either, so when the result is zero the first zero in the span is returned.That second point is not theoretical: before I added it, fuzzing found 5 cases in 80,000 where the vectorized result was
-0.0and the sequential one+0.0, which is observable throughdouble.IsNegativeand division.Maxis left alone in this PR. Its span path skips leading NaNs, returns the last element when every element is NaN, and then ignores NaNs —Vector.Maxpropagates them instead, so it needs a different shape (substituting negative infinity for NaN lanes) and deserves its own change with its own measurements.Measurements
Standalone harness (I could not build the runtime on this machine), Apple M-series arm64,
Vector128. Minimum of 7 rounds, ns per call:floatgains more because a 128-bit vector holds four of them against two doubles.Correctness
Differential against the current implementation in the harness: 80,000 cases over
floatanddouble, lengths 1–120, with NaN,+0.0,-0.0, both infinities and random values seeded into the data — 0 value mismatches and 0 signed-zero differences, the latter only after the zero handling described above was added.I have not run the System.Linq test suite locally for the reason above, so CI is the first full validation.