From 46c7f78bf7932bc79d516d37f30ae6b08845b189 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 25 Apr 2026 00:17:45 +0200 Subject: [PATCH 1/4] Use safe spans in BigInteger shift helpers Rewrite the vectorized in-place shift helpers to consume Span slices and Vector*.Create/CopyTo instead of ref arithmetic with LoadUnsafe/StoreUnsafe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Numerics/BigIntegerCalculator.ShiftRot.cs | 73 ++++++++++--------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs index 0ea6e11b122f5d..5a39adf1bca80d 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Runtime.InteropServices; using System.Runtime.Intrinsics; namespace System.Numerics @@ -115,46 +114,48 @@ public static void LeftShiftSelf(Span bits, int shift, out nuint carry) { carry = bits[^1] >> back; - ref nuint start = ref MemoryMarshal.GetReference(bits); - int offset = bits.Length; + Span remaining = bits; // Each vector load needs one extra element below it to source the carry bits // that shift in from the lower limbs, hence the +1 minimum. - while (Vector512.IsHardwareAccelerated && offset >= Vector512.Count + 1) + while (Vector512.IsHardwareAccelerated && remaining.Length >= Vector512.Count + 1) { - Vector512 current = Vector512.LoadUnsafe(ref start, (nuint)(offset - Vector512.Count)) << shift; - Vector512 carries = Vector512.LoadUnsafe(ref start, (nuint)(offset - (Vector512.Count + 1))) >> back; + int offset = remaining.Length - Vector512.Count; + Vector512 current = Vector512.Create(remaining.Slice(offset)) << shift; + Vector512 carries = Vector512.Create(remaining.Slice(offset - 1)) >> back; Vector512 newValue = current | carries; - Vector512.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector512.Count)); - offset -= Vector512.Count; + newValue.CopyTo(remaining.Slice(offset)); + remaining = remaining.Slice(0, offset); } - while (Vector256.IsHardwareAccelerated && offset >= Vector256.Count + 1) + while (Vector256.IsHardwareAccelerated && remaining.Length >= Vector256.Count + 1) { - Vector256 current = Vector256.LoadUnsafe(ref start, (nuint)(offset - Vector256.Count)) << shift; - Vector256 carries = Vector256.LoadUnsafe(ref start, (nuint)(offset - (Vector256.Count + 1))) >> back; + int offset = remaining.Length - Vector256.Count; + Vector256 current = Vector256.Create(remaining.Slice(offset)) << shift; + Vector256 carries = Vector256.Create(remaining.Slice(offset - 1)) >> back; Vector256 newValue = current | carries; - Vector256.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector256.Count)); - offset -= Vector256.Count; + newValue.CopyTo(remaining.Slice(offset)); + remaining = remaining.Slice(0, offset); } - while (Vector128.IsHardwareAccelerated && offset >= Vector128.Count + 1) + while (Vector128.IsHardwareAccelerated && remaining.Length >= Vector128.Count + 1) { - Vector128 current = Vector128.LoadUnsafe(ref start, (nuint)(offset - Vector128.Count)) << shift; - Vector128 carries = Vector128.LoadUnsafe(ref start, (nuint)(offset - (Vector128.Count + 1))) >> back; + int offset = remaining.Length - Vector128.Count; + Vector128 current = Vector128.Create(remaining.Slice(offset)) << shift; + Vector128 carries = Vector128.Create(remaining.Slice(offset - 1)) >> back; Vector128 newValue = current | carries; - Vector128.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector128.Count)); - offset -= Vector128.Count; + newValue.CopyTo(remaining.Slice(offset)); + remaining = remaining.Slice(0, offset); } nuint carry2 = 0; - for (int i = 0; i < offset; i++) + for (int i = 0; i < remaining.Length; i++) { nuint value = carry2 | bits[i] << shift; carry2 = bits[i] >> back; @@ -189,43 +190,43 @@ public static void RightShiftSelf(Span bits, int shift, out nuint carry) { carry = bits[0] << back; - ref nuint start = ref MemoryMarshal.GetReference(bits); - int offset = 0; + Span remaining = bits; - while (Vector512.IsHardwareAccelerated && bits.Length - offset >= Vector512.Count + 1) + while (Vector512.IsHardwareAccelerated && remaining.Length >= Vector512.Count + 1) { - Vector512 current = Vector512.LoadUnsafe(ref start, (nuint)offset) >> shift; - Vector512 carries = Vector512.LoadUnsafe(ref start, (nuint)(offset + 1)) << back; + Vector512 current = Vector512.Create(remaining) >> shift; + Vector512 carries = Vector512.Create(remaining.Slice(1)) << back; Vector512 newValue = current | carries; - Vector512.StoreUnsafe(newValue, ref start, (nuint)offset); - offset += Vector512.Count; + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector512.Count); } - while (Vector256.IsHardwareAccelerated && bits.Length - offset >= Vector256.Count + 1) + while (Vector256.IsHardwareAccelerated && remaining.Length >= Vector256.Count + 1) { - Vector256 current = Vector256.LoadUnsafe(ref start, (nuint)offset) >> shift; - Vector256 carries = Vector256.LoadUnsafe(ref start, (nuint)(offset + 1)) << back; + Vector256 current = Vector256.Create(remaining) >> shift; + Vector256 carries = Vector256.Create(remaining.Slice(1)) << back; Vector256 newValue = current | carries; - Vector256.StoreUnsafe(newValue, ref start, (nuint)offset); - offset += Vector256.Count; + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector256.Count); } - while (Vector128.IsHardwareAccelerated && bits.Length - offset >= Vector128.Count + 1) + while (Vector128.IsHardwareAccelerated && remaining.Length >= Vector128.Count + 1) { - Vector128 current = Vector128.LoadUnsafe(ref start, (nuint)offset) >> shift; - Vector128 carries = Vector128.LoadUnsafe(ref start, (nuint)(offset + 1)) << back; + Vector128 current = Vector128.Create(remaining) >> shift; + Vector128 carries = Vector128.Create(remaining.Slice(1)) << back; Vector128 newValue = current | carries; - Vector128.StoreUnsafe(newValue, ref start, (nuint)offset); - offset += Vector128.Count; + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector128.Count); } nuint carry2 = 0; + int offset = bits.Length - remaining.Length; for (int i = bits.Length - 1; i >= offset; i--) { nuint value = carry2 | bits[i] >> shift; From 5ac8ed9fa25a8cf86e84d877437b3b40d710ba47 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 25 Apr 2026 02:16:59 +0200 Subject: [PATCH 2/4] Limit BigInteger safe span rewrite to RightShiftSelf Restore LeftShiftSelf to the main-branch implementation so PR 127409 only changes RightShiftSelf. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Numerics/BigIntegerCalculator.ShiftRot.cs | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs index 5a39adf1bca80d..0840df4ad516d7 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Runtime.InteropServices; using System.Runtime.Intrinsics; namespace System.Numerics @@ -114,48 +115,46 @@ public static void LeftShiftSelf(Span bits, int shift, out nuint carry) { carry = bits[^1] >> back; - Span remaining = bits; + ref nuint start = ref MemoryMarshal.GetReference(bits); + int offset = bits.Length; // Each vector load needs one extra element below it to source the carry bits // that shift in from the lower limbs, hence the +1 minimum. - while (Vector512.IsHardwareAccelerated && remaining.Length >= Vector512.Count + 1) + while (Vector512.IsHardwareAccelerated && offset >= Vector512.Count + 1) { - int offset = remaining.Length - Vector512.Count; - Vector512 current = Vector512.Create(remaining.Slice(offset)) << shift; - Vector512 carries = Vector512.Create(remaining.Slice(offset - 1)) >> back; + Vector512 current = Vector512.LoadUnsafe(ref start, (nuint)(offset - Vector512.Count)) << shift; + Vector512 carries = Vector512.LoadUnsafe(ref start, (nuint)(offset - (Vector512.Count + 1))) >> back; Vector512 newValue = current | carries; - newValue.CopyTo(remaining.Slice(offset)); - remaining = remaining.Slice(0, offset); + Vector512.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector512.Count)); + offset -= Vector512.Count; } - while (Vector256.IsHardwareAccelerated && remaining.Length >= Vector256.Count + 1) + while (Vector256.IsHardwareAccelerated && offset >= Vector256.Count + 1) { - int offset = remaining.Length - Vector256.Count; - Vector256 current = Vector256.Create(remaining.Slice(offset)) << shift; - Vector256 carries = Vector256.Create(remaining.Slice(offset - 1)) >> back; + Vector256 current = Vector256.LoadUnsafe(ref start, (nuint)(offset - Vector256.Count)) << shift; + Vector256 carries = Vector256.LoadUnsafe(ref start, (nuint)(offset - (Vector256.Count + 1))) >> back; Vector256 newValue = current | carries; - newValue.CopyTo(remaining.Slice(offset)); - remaining = remaining.Slice(0, offset); + Vector256.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector256.Count)); + offset -= Vector256.Count; } - while (Vector128.IsHardwareAccelerated && remaining.Length >= Vector128.Count + 1) + while (Vector128.IsHardwareAccelerated && offset >= Vector128.Count + 1) { - int offset = remaining.Length - Vector128.Count; - Vector128 current = Vector128.Create(remaining.Slice(offset)) << shift; - Vector128 carries = Vector128.Create(remaining.Slice(offset - 1)) >> back; + Vector128 current = Vector128.LoadUnsafe(ref start, (nuint)(offset - Vector128.Count)) << shift; + Vector128 carries = Vector128.LoadUnsafe(ref start, (nuint)(offset - (Vector128.Count + 1))) >> back; Vector128 newValue = current | carries; - newValue.CopyTo(remaining.Slice(offset)); - remaining = remaining.Slice(0, offset); + Vector128.StoreUnsafe(newValue, ref start, (nuint)(offset - Vector128.Count)); + offset -= Vector128.Count; } nuint carry2 = 0; - for (int i = 0; i < remaining.Length; i++) + for (int i = 0; i < offset; i++) { nuint value = carry2 | bits[i] << shift; carry2 = bits[i] >> back; From b30f3f8d465263e1316f0a10a71c490b8e98f5e3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 25 Apr 2026 02:47:32 +0200 Subject: [PATCH 3/4] feedback --- .../Numerics/BigIntegerCalculator.ShiftRot.cs | 77 ++++++++----------- 1 file changed, 32 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs index 0840df4ad516d7..0df964fd4208fd 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs @@ -185,63 +185,50 @@ public static void RightShiftSelf(Span bits, int shift, out nuint carry) int back = BitsPerLimb - shift; - if (Vector128.IsHardwareAccelerated) - { - carry = bits[0] << back; + carry = bits[0] << back; - Span remaining = bits; - - while (Vector512.IsHardwareAccelerated && remaining.Length >= Vector512.Count + 1) - { - Vector512 current = Vector512.Create(remaining) >> shift; - Vector512 carries = Vector512.Create(remaining.Slice(1)) << back; + Span remaining = bits; - Vector512 newValue = current | carries; + while (Vector512.IsHardwareAccelerated && remaining.Length >= Vector512.Count + 1) + { + Vector512 current = Vector512.Create(remaining) >> shift; + Vector512 carries = Vector512.Create(remaining.Slice(1)) << back; - newValue.CopyTo(remaining); - remaining = remaining.Slice(Vector512.Count); - } + Vector512 newValue = current | carries; - while (Vector256.IsHardwareAccelerated && remaining.Length >= Vector256.Count + 1) - { - Vector256 current = Vector256.Create(remaining) >> shift; - Vector256 carries = Vector256.Create(remaining.Slice(1)) << back; + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector512.Count); + } - Vector256 newValue = current | carries; + while (Vector256.IsHardwareAccelerated && remaining.Length >= Vector256.Count + 1) + { + Vector256 current = Vector256.Create(remaining) >> shift; + Vector256 carries = Vector256.Create(remaining.Slice(1)) << back; - newValue.CopyTo(remaining); - remaining = remaining.Slice(Vector256.Count); - } + Vector256 newValue = current | carries; - while (Vector128.IsHardwareAccelerated && remaining.Length >= Vector128.Count + 1) - { - Vector128 current = Vector128.Create(remaining) >> shift; - Vector128 carries = Vector128.Create(remaining.Slice(1)) << back; + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector256.Count); + } - Vector128 newValue = current | carries; + while (Vector128.IsHardwareAccelerated && remaining.Length >= Vector128.Count + 1) + { + Vector128 current = Vector128.Create(remaining) >> shift; + Vector128 carries = Vector128.Create(remaining.Slice(1)) << back; - newValue.CopyTo(remaining); - remaining = remaining.Slice(Vector128.Count); - } + Vector128 newValue = current | carries; - nuint carry2 = 0; - int offset = bits.Length - remaining.Length; - for (int i = bits.Length - 1; i >= offset; i--) - { - nuint value = carry2 | bits[i] >> shift; - carry2 = bits[i] << back; - bits[i] = value; - } + newValue.CopyTo(remaining); + remaining = remaining.Slice(Vector128.Count); } - else + + nuint carry2 = 0; + int offset = bits.Length - remaining.Length; + for (int i = bits.Length - 1; i >= offset; i--) { - carry = 0; - for (int i = bits.Length - 1; i >= 0; i--) - { - nuint value = carry | bits[i] >> shift; - carry = bits[i] << back; - bits[i] = value; - } + nuint value = carry2 | bits[i] >> shift; + carry2 = bits[i] << back; + bits[i] = value; } } } From 7120314a29170f0ab775f78cf4fd6a4d907f3ceb Mon Sep 17 00:00:00 2001 From: EgorBo <523221+EgorBo@users.noreply.github.com> Date: Sat, 25 Apr 2026 03:19:27 +0200 Subject: [PATCH 4/4] Address PR feedback: rewrite RightShiftSelf tail loop in remaining-coordinates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Numerics/BigIntegerCalculator.ShiftRot.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs index 0df964fd4208fd..af8fd82879585d 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.ShiftRot.cs @@ -222,14 +222,11 @@ public static void RightShiftSelf(Span bits, int shift, out nuint carry) remaining = remaining.Slice(Vector128.Count); } - nuint carry2 = 0; - int offset = bits.Length - remaining.Length; - for (int i = bits.Length - 1; i >= offset; i--) + for (int i = 0; i < remaining.Length - 1; i++) { - nuint value = carry2 | bits[i] >> shift; - carry2 = bits[i] << back; - bits[i] = value; + remaining[i] = (remaining[i] >> shift) | (remaining[i + 1] << back); } + remaining[remaining.Length - 1] >>= shift; } } }