Skip to content

Compare a vector at a time when taking the maximum of a float span - #134408

Closed
tahakocal wants to merge 2 commits into
dotnet:mainfrom
tahakocal:perf/linq-max-float
Closed

tahakocal wants to merge 2 commits into
dotnet:mainfrom
tahakocal:perf/linq-max-float

Conversation

@tahakocal

Copy link
Copy Markdown

Companion to #134407, which did the same for Min.

Max over a span of float or double walks it one element at a time:

int i;
for (i = 0; i < span.Length && T.IsNaN(span[i]); i++) ;
if (i == span.Length) { return span[^1]; }

for (value = span[i]; (uint)i < (uint)span.Length; i++)
{
    if (span[i] > value) { value = span[i]; }
}

Change

Once the leading NaNs are skipped and a first candidate is in hand, the rest of the span is compared a vector at a time.

Two details of the sequential behaviour are preserved:

  • NaN. After the leading run, the walk simply ignores NaNs, because span[i] > value is false for them, while Vector128.Max would propagate one into the result. NaN lanes are therefore replaced with negative infinity before the comparison. The leading-NaN scan and the all-NaN case (which returns the last element) are untouched.
  • Signed zero. The walk keeps the first of two equal values and -0.0 == +0.0, so when the result is zero the first zero in the span is returned rather than whichever one the reduction kept.

The substitution is why this is guarded on Vector128<T>.Count >= 4, which in practice means float and not double: replacing the NaN lanes costs a compare and a select per vector, and with only two elements per vector that cancels the gain. Measured for double: 1.43x at 1K, 1.05x at 8K, 1.01x at 64K and 0.93x at a million — so double keeps the sequential walk. I would rather scope it by the measurement than ship a change that is a wash or a small regression on half the types it touches.

Measurements

Standalone harness (I could not build the runtime on this machine), Apple M-series arm64, Vector128. Minimum of 7 rounds, ns per call:

type length before after speedup
float 128 31.6 10.4 3.05x
float 1,024 271.4 92.2 2.94x
float 8,192 1,937.3 890.2 2.18x
float 65,536 15,415.4 7,432.8 2.07x
float 1,000,000 227,798 115,782 1.97x
double (not taken) 1,000,000 241,820 259,410 0.93x

Correctness

Differential against the current implementation in the harness: 80,080 cases over float and double, lengths 1–120, with NaN, +0.0, -0.0, both infinities and random values seeded in, plus every all-NaN span and every leading-NaN span from length 1 to 40 — 0 value mismatches and 0 signed-zero differences.

I have not run the System.Linq test suite locally for the reason above, so CI is the first full validation.

Max over a span of float or double walks it one element at a time. Compare a
vector at a time instead, once the leading NaNs have been skipped and a first
candidate is in hand.

NaN lanes are replaced with negative infinity before the comparison, because
the sequential walk ignores a NaN that is not the leading one while
Vector128.Max would propagate it. That substitution costs two operations per
vector, which only pays off when a vector holds at least four elements, so
double keeps the sequential walk: measured 0.93x to 1.01x for double against
1.97x to 3.05x for float.

The walk 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.
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 22, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

if (Vector128.IsHardwareAccelerated && Vector128<T>.IsSupported &&
Vector128<T>.Count >= 4 && span.Length - i >= Vector128<T>.Count * 2)
{
ref T first = ref MemoryMarshal.GetReference(span);

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.

Rewrite to safe code

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

@tahakocal

Copy link
Copy Markdown
Author

Switched to safe loads — Vector128.Create(span.Slice(i)) instead of MemoryMarshal.GetReference plus LoadUnsafe, and the System.Runtime.InteropServices using is gone.

Correctness re-checked on the safe form: 80,080 cases, 0 value mismatches and 0 signed-zero differences, including every all-NaN and leading-NaN span from length 1 to 40.

The float numbers come out slightly lower than the description's, which were taken on the unsafe form: 2.56x at 128, 2.67x at 1,024 and 2.10x at 8,192. I measured the two load forms side by side only for #134407, where the bounds check costs about a quarter of the gain at a thousand elements and nothing at a million — I would expect the same shape here, but I have not measured it for Max specifically and would rather say so than imply I did.

Happy to update the description to the safe-form numbers once you confirm which form you want to keep.

@EgorBo

EgorBo commented Sep 22, 2026

Copy link
Copy Markdown
Member

where the bounds check costs about a quarter of the gain at

Note: I already commented about this in the other PR - so I highly recommend either working PR by PR or group similar chnages in one PR.

@tahakocal

Copy link
Copy Markdown
Author

Also built the runtime locally since my earlier note: System.Linq.Tests on osx-arm64 Release from this branch, with the safe loads in place: 52,260 total, 0 errors, 0 failed, 8 skipped.

@tahakocal

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Runtime community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants