Fix BigInteger.LeadingZeroCount, PopCount, TrailingZeroCount, RotateLeft, and RotateRight for platform-independent 32-bit word semantics

[Copilot]

After the _bits migration from uint[] to nuint[] (#125799), several IBinaryInteger<BigInteger> methods returned platform-dependent results. This PR restores consistent 32-bit word semantics for all five affected methods, including both the _sign path and the _bits path.

Description

Changes

LeadingZeroCount:

• _sign path: nint.LeadingZeroCount → uint.LeadingZeroCount((uint)value._sign)
• _bits path: BitOperations.LeadingZeroCount(value._bits[^1]) & 31 to map to 32-bit word semantics

PopCount:

• _sign path: nint.PopCount(value._sign) → int.PopCount(value._sign)
  • Fixes PopCount(-1) returning 64 on 64-bit vs 32 on 32-bit
• _bits negative path: Replace inline two's complement with formula PopCount(2^W - m) = W - PopCount(m) - TZC(m) + 1 using 32-bit word width W
  • Fixes ~(nuint) filling upper 32 bits with 1s on 64-bit when magnitude only uses lower 32 bits

TrailingZeroCount:

• _sign path: nint.TrailingZeroCount(value._sign) → int.TrailingZeroCount(value._sign)
  • Fixes TrailingZeroCount(BigInteger.Zero) returning 64 on 64-bit vs 32 on 32-bit

RotateLeft:

• _sign path: BitOperations.RotateLeft((nuint)value._sign, rotateAmount) → uint.RotateLeft((uint)value._sign, rotateAmount) with new BigInteger((int)rs) / new BigInteger(rs) result construction
  • Fixes RotateLeft(BigInteger.One, 32) returning 2^32 on 64-bit vs 1 on 32-bit
• _bits path: Rewrote Rotate() helper to use 32-bit word semantics via MemoryMarshal.Cast<nuint, uint>(), performing two's complement conversion, rotation, and sign-bit checks on Span<uint> instead of Span<nuint>

RotateRight:

• _sign path: BitOperations.RotateRight((nuint)value._sign, rotateAmount) → uint.RotateRight((uint)value._sign, rotateAmount) with new BigInteger((int)rs) / new BigInteger(rs) result construction
  • Fixes RotateRight(BigInteger.One, 1) returning 2^63 on 64-bit vs 2^31 on 32-bit
• _bits path: Same Rotate() rewrite as RotateLeft (shared helper)

New BigIntegerCalculator helpers:

• Added RotateLeft32 / RotateRight32 operating on Span<uint> with 32-bit shift arithmetic
• Added LeftShiftSelf32 / RightShiftSelf32 for bit-level shifts on uint spans
• Added SwapUpperAndLower32 for digit-level rotation using ArrayPool<uint>

Other methods verified: Log2 is NOT affected — the platform dependencies in BitsPerLimb and BitOperations.LeadingZeroCount cancel each other out, producing correct results on both platforms.

Tests: Comprehensive test coverage added/updated for all five methods:

• BigIntegerTests.GenericMath.cs: Updated RotateLeftTest/RotateRightTest with platform-independent assertions and additional _sign path coverage
• Rotate.cs: Fixed NegativeNumber_TestData and PowerOfTwo tests to use bpl = 32 instead of nint.Size * 8, fixed helper methods to use 4-byte alignment instead of nint.Size, fixed (nint)(-1) → (int)(-1) cast
• MyBigInt.cs: Fixed RotateLeft test reference implementation to pad to 4-byte boundaries instead of nint.Size
• All 3031 tests pass with zero failures

---

⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

[dotnet-policy-service]

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

[stephentoub]

@tannergooding, is the .NET 10 behavior that uses the upper 32-bit word what we want?

[Copilot]

Pull request overview

This PR restores platform-independent, 32-bit word–based semantics for BigInteger.LeadingZeroCount after the internal _bits storage migrated from uint[] to nuint[], and adds/updates tests to lock in the intended behavior.

Changes:

• Update BigInteger.LeadingZeroCount to always compute LZC using 32-bit word semantics (including on 64-bit platforms).
• Fix existing generic-math test expectations to be platform-independent (32/31 for Zero/One).
• Add a dedicated LeadingZeroCountTests suite and include it in the Numerics test project.

Reviewed changes

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

File	Description
src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs	Adjusts LeadingZeroCount to use 32-bit word semantics for both _sign and _bits representations.
src/libraries/System.Runtime.Numerics/tests/BigIntegerTests.GenericMath.cs	Updates assertions to expect platform-independent results for LeadingZeroCount(Zero/One).
src/libraries/System.Runtime.Numerics/tests/BigInteger/LeadingZeroCountTests.cs	Adds coverage for boundary and platform-independence scenarios (small/large, positive/negative).
src/libraries/System.Runtime.Numerics/tests/System.Runtime.Numerics.Tests.csproj	Includes the newly added LeadingZeroCountTests.cs in the test build.

Comments suppressed due to low confidence (1)

src/libraries/System.Runtime.Numerics/tests/BigInteger/LeadingZeroCountTests.cs:67

• Same as above: this parse call would be clearer and more consistent with the surrounding test suite if it used using System.Globalization; + NumberStyles.HexNumber instead of the Globalization. qualifier.

            // Parse the magnitude as a positive hex value (leading zero keeps high bit clear),
            // then negate it so the result is negative and stored in _bits.
            BigInteger magnitude = BigInteger.Parse(hexMagnitude, Globalization.NumberStyles.HexNumber);
            BigInteger value = -magnitude;

[github-actions]

🤖 Copilot Code Review — PR #126259

Note

This review was AI-generated by GitHub Copilot CLI (Claude Opus 4.6).

Holistic Assessment

Motivation: Justified. After #125799 migrated _bits from uint[] to nuint[], LeadingZeroCount, PopCount, and TrailingZeroCount started returning platform-dependent results (e.g., PopCount(-1) = 64 on 64-bit vs 32 on 32-bit, TrailingZeroCount(0) = 64 vs 32). This is a real regression for an arbitrary-precision type.

Approach: Sound. The fix standardizes on 32-bit word semantics using three clean strategies: (1) fixed-width int/uint operations for _sign paths, (2) a & 31 bitmask trick for LeadingZeroCount's _bits path, and (3) a mathematical identity for PopCount's negative _bits path. Each avoids the fundamental problem of nint/nuint-width operations giving platform-dependent results.

Summary: ✅ LGTM. The code changes are correct. All three methods are fixed with clean, well-commented implementations. Tests are comprehensive, covering _sign and _bits paths, positive/negative values, boundary cases, and platform-independence invariants. Maintainer feedback (jkotas's & 31 suggestion, stephentoub's review) has been incorporated. The only follow-up observation (RotateLeft/RotateRight) is a pre-existing issue and out of scope.

---

Detailed Findings

✅ Correctness — LeadingZeroCount & 31 trick is correct

The expression BitOperations.LeadingZeroCount(value._bits[^1]) & 31 correctly maps 64-bit LZC to 32-bit word semantics:

Scenario (64-bit)	MSL value	LZC	& 31	32-bit equivalent
Upper half non-zero (e.g., 2⁶³)	0x80000000_00000000	0	0	✅
Upper half = 1 (e.g., 2³²)	0x00000001_00000000	31	31	✅
Upper half zero (e.g., 2³¹)	0x00000000_80000000	32	0	✅
Lower = 1 only	0x00000000_00000001	63	31	✅

This relies on BigInteger's normalization invariant that _bits[^1] != 0, which is well-established and maintained throughout the codebase. On 32-bit, LZC is 0–31 and & 31 is a no-op. The comment at line 3168–3169 explains the trick clearly.

✅ Correctness — PopCount negative path formula is correct

The mathematical identity PopCount(2^W - m) = W - PopCount(m) - TZC(m) + 1 is correct and avoids the root cause: ~(nuint) on 64-bit fills upper 32 bits with 1s when the magnitude only uses lower 32 bits.

I verified with concrete examples:

• -2³¹ (magnitude 0x80000000): On 64-bit, magnitudePC=1, magnitudeTZC=31, wordCount=1 (MSL upper half=0), W=32 → result=32-1-31+1=1 ✓
• -(2³²) (magnitude 0x100000000): On 64-bit, magnitudePC=1, magnitudeTZC=32, wordCount=2, W=64 → result=64-1-32+1=32 ✓
• -1 (via _sign path): int.PopCount(-1) = PopCount(0xFFFFFFFF) = 32 ✓

The old code had a real bug: ~0x0000000080000000uL = 0xFFFFFFFF7FFFFFFF → PopCount=63, but the correct 32-bit answer for ~0x80000000u is PopCount=31.

✅ Correctness — _sign path fixes are straightforward

• LeadingZeroCount: nint.LeadingZeroCount → uint.LeadingZeroCount((uint)value._sign) — always 32-bit width.
• PopCount: nint.PopCount → int.PopCount — 32-bit two's complement PopCount.
• TrailingZeroCount: nint.TrailingZeroCount → int.TrailingZeroCount — fixes TZC(0) from 64→32 on 64-bit.

All three are clean substitutions that lock in 32-bit semantics regardless of platform.

✅ Correctness — Sibling methods are already platform-consistent

I checked the remaining IBinaryInteger<BigInteger> methods for similar platform-dependency issues:

• GetShortestBitLength: Uses BitsPerLimb consistently in its calculations. Verified: for value 2³² as negative, result is 33 on both platforms. ✅
• Log2: Uses BitsPerLimb in its XOR-based formula. Verified: Log2(2³²) = 32 on both platforms. ✅
• TrailingZeroCount _bits path (unchanged): TZC is an intrinsic property of the magnitude's binary representation, independent of limb width. ✅
• PopCount positive _bits path (unchanged): PopCount of zero upper bits contributes 0, so nuint-width PopCount matches 32-bit. ✅

✅ Test quality — comprehensive and well-structured

Tests cover all critical dimensions:

• _sign path (small values in int range) and _bits path (large values)
• Positive and negative values
• Boundary values at 32-bit/64-bit word boundaries (2³¹, 2³², 2⁶³, 2⁶⁴, 2¹²⁷, 2¹²⁸)
• Platform-independence invariants explicitly tested
• Tests are merged into existing BigIntegerTests.GenericMath.cs methods following the established pattern

💡 Observation — RotateLeft/RotateRight have a similar pre-existing issue

The _sign path in RotateLeft (line 3259) and RotateRight (line 3280) uses BitOperations.RotateLeft/Right((nuint)value._sign, ...), which rotates within 32-bit vs 64-bit width depending on platform. This is a pre-existing issue not introduced by this PR and is out of scope, but worth noting as a potential follow-up for platform-consistency.

Generated by Code Review for issue #126259 · ◷

[stephentoub]

@copilot also address RotateLeft/RotateRight and any other similarly affected methods on BigInteger. Ensure comprehensive tests are added/updated in a manner consistent with the other APIs in this PR.

[Copilot]

@copilot also address RotateLeft/RotateRight and any other similarly affected methods on BigInteger. Ensure comprehensive tests are added/updated in a manner consistent with the other ...

Fixed RotateLeft and RotateRight _sign paths in d9749d7 — replaced BitOperations.RotateLeft/Right((nuint)value._sign, ...) with uint.RotateLeft/Right((uint)value._sign, ...) and updated result construction to use (int)rs/(uint)rs. Updated all platform-dependent test assertions (nint.Size * 8 expressions → fixed 32-bit values) and added comprehensive tests for _sign path values.

I also verified that Log2 is NOT affected — the platform dependencies in BitsPerLimb and BitOperations.LeadingZeroCount cancel each other out, producing correct results on both platforms.

Note: The pre-existing Rotate.cs tests (RunSmallTests, RunRotateTests) fail because the _bits path Rotate() helper also has platform-dependent behavior when operating on nuint[] limbs (e.g., two's complement conversion sign-extends differently on 64-bit). This is a broader issue separate from the _sign path fixes in this PR.

[stephentoub]

Note: The pre-existing Rotate.cs tests (RunSmallTests, RunRotateTests) fail because the _bits path Rotate() helper also has platform-dependent behavior when operating on nuint[] limbs (e.g., two's complement conversion sign-extends differently on 64-bit). This is a broader issue separate from the _sign path fixes in this PR.

@copilot, can you elaborate? If it wasn't platform-dependent before #125799, then I need you to fix it in this PR.

[Copilot]

Note: The pre-existing Rotate.cs tests (RunSmallTests, RunRotateTests) fail because the _bits path Rotate() helper also has platform-dependent behavior when operating on nuint[] limbs (e.g., two's c...

Fixed in b52efac and b57835d. The _bits path Rotate() helper was indeed broken by #125799 — on 64-bit, each nuint limb is 64 bits wide, so the rotation ring width doubled for values where the MSW's upper 32 bits were zero (e.g., 0x80000000 stored in 1 nuint = 64-bit rotation vs 1 uint = 32-bit rotation previously).

The fix reinterprets the nuint[] buffer as a Span<uint> via MemoryMarshal.Cast and performs all rotation logic on 32-bit words:

• New RotateLeft32/RotateRight32 helpers in BigIntegerCalculator operate on Span<uint> with 32-bit shift arithmetic
• Two's complement conversion and sign-bit checks use 32-bit word semantics
• MyBigInt.RotateLeft test reference fixed to pad to 4-byte boundaries (was nint.Size)
• All Rotate.cs test data updated from nint.Size * 8 to 32

All 3031 tests pass.