Skip to content

Replace ExtractMostSignificantBits+BitOp patterns with Vector helper methods#126841

Closed
Copilot wants to merge 11 commits into
mainfrom
copilot/replace-extract-msb-with-vector-functions
Closed

Replace ExtractMostSignificantBits+BitOp patterns with Vector helper methods#126841
Copilot wants to merge 11 commits into
mainfrom
copilot/replace-extract-msb-with-vector-functions

Conversation

Copilot AI commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Replace usages of ExtractMostSignificantBits() followed by PopCount/TrailingZeroCount/LeadingZeroCount with the recently optimized [Intrinsic] vector helpers: CountMatches, IndexOfFirstMatch, IndexOfLastMatch, and IndexOfWhereAllBitsSet.

Description

Pattern replacements

  • BitOperations.PopCount(v.ExtractMostSignificantBits())VectorN.CountMatches(v) (within CoreLib, using internal helper to avoid x64 regression)
  • BitOperations.TrailingZeroCount(v.ExtractMostSignificantBits())VectorN.IndexOfFirstMatch(v) (within CoreLib) or VectorN.IndexOfWhereAllBitsSet(v) (in Tensors, using public API)
  • Count - 1 - BitOperations.LeadingZeroCount(v.ExtractMostSignificantBits())VectorN.IndexOfLastMatch(v) (within CoreLib)
-count += BitOperations.PopCount(Vector512.Equals(Vector512.LoadUnsafe(ref current), targetVector).ExtractMostSignificantBits());
+count += Vector512.CountMatches(Vector512.Equals(Vector512.LoadUnsafe(ref current), targetVector));
-uint matches = Vector128.Equals(Vector128<byte>.Zero, search).ExtractMostSignificantBits();
-if (matches == 0)
+Vector128<byte> cmp = Vector128.Equals(Vector128<byte>.Zero, search);
+if (cmp == Vector128<byte>.Zero)
 {
     offset += (nuint)Vector128<byte>.Count;
 }
 else
 {
-    return (int)(offset + (uint)BitOperations.TrailingZeroCount(matches));
+    return (int)(offset + (uint)Vector128.IndexOfFirstMatch(cmp));
 }

Files changed

  • SpanHelpers.T.cs — 3 replacements in CountValueType loop bodies using the internal CountMatches helper directly; replaced ComputeFirstIndex (3 overloads, EMSB+TZCIndexOfFirstMatch) and ComputeLastIndex (3 overloads, EMSB+LZCIndexOfLastMatch)
  • SpanHelpers.Byte.cs — 9 replacements in the null-terminator search (EMSB+TZCIndexOfFirstMatch), keeping the vector comparison result and comparing against VectorN<byte>.Zero instead of extracting to a bitmask
  • TensorPrimitives.Max.cs — Inlined VectorN.IndexOfWhereAllBitsSet directly at all 9 call sites (previously consumed the shared IndexOfFirstMatch helpers)

Not changed

  • Vector64.cs / Vector128.cs — Left as-is; these contain the internal helper methods (CountMatches, IndexOfFirstMatch, etc.) that the other callsites consume
  • TensorPrimitives.IndexOfMax.cs — Left as-is; no changes made to this file
  • TensorPrimitives.HammingDistance.cs — Left as-is; CountWhereAllBitsSet would introduce an x64 regression, and the internal CountMatches helper is not accessible from the Tensors assembly
  • SpanHelpers.Byte.cs loop patterns — Left as-is; IndexOf/LastIndexOf patterns that iterate through multiple matches with ResetLowestSetBit/FlipBit don't map to single-match helpers
  • SpanHelpers.Byte.cs SequenceEqual — Left as-is; computes TZC(~matches) for first difference, not first match

Testing

All relevant tests pass:

  • System.Runtime.Intrinsics.Tests: 12,944 tests passed
  • System.Memory.Tests: 52,229 tests passed
  • System.Numerics.Tensors.Tests: 5,427 tests passed
  • System.Runtime.Tests: 69,682 tests passed

Replace patterns of ExtractMostSignificantBits() followed by
PopCount/TrailingZeroCount/LeadingZeroCount with the optimized
Vector helpers: CountWhereAllBitsSet, IndexOfWhereAllBitsSet,
and LastIndexOfWhereAllBitsSet.

Remove AdvSimd special paths from Vector64/Vector128 internal
helpers (CountMatches, IndexOfFirstMatch, IndexOfLastMatch) and
the now-unused AdvSimdExtractBitMask/AdvSimdFixupBitCount methods.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fcd7f0bb-7e64-41fa-8773-089705b9a737

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Comment thread src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs Outdated
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

…ctly

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fd3915a5-bbf3-4530-8406-f3983be66ffb

Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 17:34
Copilot AI requested a review from tannergooding April 13, 2026 17:36
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/527f6141-5f1f-41b3-bb40-336677a9c787

Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 18:02
…ression

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/83aad277-6910-4c67-8aac-88c2e15047b3

Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 18:37
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/83aad277-6910-4c67-8aac-88c2e15047b3

Co-authored-by: tannergooding <10487869+tannergooding@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 13, 2026 18:38
@EgorBo

EgorBo commented Jun 24, 2026

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

…ct-msb-with-vector-functions

# Conflicts:
#	src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/netcore/TensorPrimitives.IndexOfMax.cs

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 24, 2026 13:28

Copilot AI commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in 942e23e. The only conflict was in TensorPrimitives.IndexOfMax.cs where main still had the old IndexOfFirstMatch helper methods that this PR removed — kept our version (helpers removed, IndexOfWhereAllBitsSet inlined directly).

Copilot AI review requested due to automatic review settings June 24, 2026 23:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment on lines +134 to +138
private static unsafe int IndexOfMinMaxCore<T, TIndexOfMinMax>(ReadOnlySpan<T> x)
where T : INumber<T>
where TIndexOfMinMax : struct, IIndexOfOperator<T>
{
if (x.IsEmpty)
Comment on lines 27 to 33
public static int IndexOfMax<T>(ReadOnlySpan<T> x)
where T : INumber<T> =>
IndexOfMinMaxCore<T, IndexOfMaxOperator<T>>(x);

/// <summary>Returns the index of MathF.Max(x, y)</summary>
internal readonly struct IndexOfMaxOperator<T> : IIndexOfMinMaxOperator<T> where T : INumber<T>
internal readonly struct IndexOfMaxOperator<T> : IIndexOfOperator<T> where T : INumber<T>
{
@EgorBo

EgorBo commented Jun 24, 2026

Copy link
Copy Markdown
Member

@copilot please completely revert all changes in TensorPrimitives.IndexOfMax.cs

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please completely revert all changes in TensorPrimitives.IndexOfMax.cs

Done — TensorPrimitives.IndexOfMax.cs has been fully reverted to its original state (identical to main) in the latest commit.

@EgorBo

EgorBo commented Jun 25, 2026

Copy link
Copy Markdown
Member

@tannergooding feel free to pick it up, but looks like #129688 will optimize all uses.

@EgorBo EgorBo closed this Jun 25, 2026
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.

4 participants