Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor cleanups for System.Runtime.Numerics #53984

Merged
merged 10 commits into from
Aug 2, 2021
Merged

Minor cleanups for System.Runtime.Numerics #53984

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a31cb61
Replace union with BitConverter
huoyaoyuan Jun 9, 2021
e3108be
Replace CombineHash with HashCode
huoyaoyuan Jun 9, 2021
2f79a0a
Replace CbitHighZero with LeadingZeroCount
huoyaoyuan Jun 9, 2021
4bfe2ff
Remove another LeadingZeros
huoyaoyuan Jun 9, 2021
86b08e5
More cleanup in BigInteger
huoyaoyuan Jun 10, 2021
7e37a19
Cleanup complex
huoyaoyuan Jun 10, 2021
3022198
Apply suggestions from code review
huoyaoyuan Jun 10, 2021
b450c7a
Convert body-less for to while
huoyaoyuan Jun 10, 2021
d48fa75
Merge branch 'main' into numerics-cleanup
huoyaoyuan Jun 16, 2021
c2864ab
Use HashCode.AddBytes
huoyaoyuan Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 27 additions & 28 deletions src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public BigInteger(ReadOnlySpan<byte> value, bool isUnsigned = false, bool isBigE
bool isNegative;
if (byteCount > 0)
{
byte mostSignificantByte = isBigEndian ? value[0] : value[byteCount - 1];
byte mostSignificantByte = isBigEndian ? value[0] : value[^1];
Copy link
Member

Choose a reason for hiding this comment

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

Does this impact codegen at all? I could imagine this being equivalent to value.Length - 1, which might result in a secondary read from memory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Given the size of this method, I think 1 length load should be OK

Copy link
Member

Choose a reason for hiding this comment

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

Probably and for consistency, I'd just like to know if it is impactful at all, so we can more readily pin-point any changes in the perf issue triage.

isNegative = (mostSignificantByte & 0x80) != 0 && !isUnsigned;

if (mostSignificantByte == 0)
Expand Down Expand Up @@ -639,12 +639,12 @@ public bool IsPowerOfTwo
AssertValid();

if (_bits == null)
return (_sign & (_sign - 1)) == 0 && _sign != 0;
return BitOperations.IsPow2(_sign);

if (_sign != 1)
return false;
int iu = _bits.Length - 1;
if ((_bits[iu] & (_bits[iu] - 1)) != 0)
if (!BitOperations.IsPow2(_bits[iu]))
return false;
while (--iu >= 0)
{
Expand Down Expand Up @@ -818,12 +818,12 @@ public static double Log(BigInteger value, double baseValue)
if (value._bits == null)
return Math.Log(value._sign, baseValue);

ulong h = value._bits[value._bits.Length - 1];
ulong m = value._bits.Length > 1 ? value._bits[value._bits.Length - 2] : 0;
ulong l = value._bits.Length > 2 ? value._bits[value._bits.Length - 3] : 0;
ulong h = value._bits[^1];
ulong m = value._bits.Length > 1 ? value._bits[^2] : 0;
ulong l = value._bits.Length > 2 ? value._bits[^3] : 0;

// Measure the exact bit count
int c = NumericsHelpers.CbitHighZero((uint)h);
int c = BitOperations.LeadingZeroCount((uint)h);
long b = (long)value._bits.Length * 32 - c;

// Extract most significant bits
Expand Down Expand Up @@ -989,19 +989,18 @@ public override int GetHashCode()

if (_bits == null)
return _sign;
int hash = _sign;

HashCode hash = default;
hash.Add(_sign);
for (int iv = _bits.Length; --iv >= 0;)
hash = NumericsHelpers.CombineHash(hash, unchecked((int)_bits[iv]));
return hash;
hash.Add(_bits[iv]);
return hash.ToHashCode();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

public override bool Equals([NotNullWhen(true)] object? obj)
{
AssertValid();

if (!(obj is BigInteger))
return false;
return Equals((BigInteger)obj);
return obj is BigInteger bigInt && Equals(bigInt);
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
}

public bool Equals(long other)
Expand Down Expand Up @@ -1124,9 +1123,9 @@ public int CompareTo(object? obj)
{
if (obj == null)
return 1;
if (!(obj is BigInteger))
if (obj is not BigInteger bigInt)
throw new ArgumentException(SR.Argument_MustBeBigInt, nameof(obj));
return CompareTo((BigInteger)obj);
return CompareTo(bigInt);
}

/// <summary>
Expand Down Expand Up @@ -1294,13 +1293,13 @@ private enum GetBytesMode { AllocateArray, Count, Span }
// because a bits array of all zeros would represent 0, and this case
// would be encoded as _bits = null and _sign = 0.
Debug.Assert(bits.Length > 0);
Debug.Assert(bits[bits.Length - 1] != 0);
Debug.Assert(bits[^1] != 0);
while (bits[nonZeroDwordIndex] == 0U)
{
nonZeroDwordIndex++;
}

highDword = ~bits[bits.Length - 1];
highDword = ~bits[^1];
if (bits.Length - 1 == nonZeroDwordIndex)
{
// This will not overflow because highDword is less than or equal to uint.MaxValue - 1.
Expand All @@ -1312,7 +1311,7 @@ private enum GetBytesMode { AllocateArray, Count, Span }
{
Debug.Assert(sign == 1);
highByte = 0x00;
highDword = bits[bits.Length - 1];
highDword = bits[^1];
}

byte msb;
Expand Down Expand Up @@ -1471,14 +1470,14 @@ private ReadOnlySpan<uint> ToUInt32Span(Span<uint> scratch)

// Find highest significant byte and ensure high bit is 0 if positive, 1 if negative
int msb;
for (msb = dwords.Length - 1; msb > 0 && dwords[msb] == highDWord; msb--);
for (msb = dwords.Length - 1; msb > 0 && dwords[msb] == highDWord; msb--) ;
jeffhandley marked this conversation as resolved.
Show resolved Hide resolved
bool needExtraByte = (dwords[msb] & 0x80000000) != (highDWord & 0x80000000);

int length = msb + 1 + (needExtraByte ? 1 : 0);
bool copyDwordsToScratch = true;
if (length <= scratch.Length)
{
scratch = scratch.Slice(0, length);
scratch = scratch[..length];
copyDwordsToScratch = !dwordsIsScratch;
}
else
Expand All @@ -1488,7 +1487,7 @@ private ReadOnlySpan<uint> ToUInt32Span(Span<uint> scratch)

if (copyDwordsToScratch)
{
dwords.Slice(0, msb + 1).CopyTo(scratch);
dwords[..(msb + 1)].CopyTo(scratch);
}

if (needExtraByte)
Expand Down Expand Up @@ -1818,11 +1817,11 @@ public static explicit operator double(BigInteger value)
return double.NegativeInfinity;
}

ulong h = bits[length - 1];
ulong m = length > 1 ? bits[length - 2] : 0;
ulong l = length > 2 ? bits[length - 3] : 0;
ulong h = bits[^1];
ulong m = length > 1 ? bits[^2] : 0;
ulong l = length > 2 ? bits[^3] : 0;

int z = NumericsHelpers.CbitHighZero((uint)h);
int z = BitOperations.LeadingZeroCount((uint)h);

int exp = (length - 2) * 32 - z;
ulong man = (h << 32 + z) | (m << z) | (l >> 32 - z);
Expand Down Expand Up @@ -2406,7 +2405,7 @@ public long GetBitLength()
else
{
bitsArrayLength = bits.Length;
highValue = bits[bitsArrayLength - 1];
highValue = bits[^1];
}

long bitLength = bitsArrayLength * 32L - BitOperations.LeadingZeroCount(highValue);
Expand Down Expand Up @@ -2483,7 +2482,7 @@ private void AssertValid()
// Wasted space: _bits[0] could have been packed into _sign
Debug.Assert(_bits.Length > 1 || _bits[0] >= kuMaskHighBit);
// Wasted space: leading zeros could have been truncated
Debug.Assert(_bits[_bits.Length - 1] != 0);
Debug.Assert(_bits[^1] != 0);
}
else
{
Expand Down
36 changes: 1 addition & 35 deletions src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private static unsafe void Divide(uint* left, int leftLength,
uint divLo = rightLength > 1 ? right[rightLength - 2] : 0;

// We measure the leading zeros of the divisor
int shift = LeadingZeros(divHi);
int shift = BitOperations.LeadingZeroCount(divHi);
int backShift = 32 - shift;

// And, we make sure the most significant bit is set
Expand Down Expand Up @@ -316,39 +316,5 @@ private static bool DivideGuessTooBig(ulong q, ulong valHi, uint valLo,

return false;
}

private static int LeadingZeros(uint value)
{
if (value == 0)
return 32;

int count = 0;
if ((value & 0xFFFF0000) == 0)
{
count += 16;
value = value << 16;
}
if ((value & 0xFF000000) == 0)
{
count += 8;
value = value << 8;
}
if ((value & 0xF0000000) == 0)
{
count += 4;
value = value << 4;
}
if ((value & 0xC0000000) == 0)
{
count += 2;
value = value << 2;
}
if ((value & 0x80000000) == 0)
{
count += 1;
}

return count;
}
}
}
2 changes: 1 addition & 1 deletion src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.GcdInv.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private static void ExtractDigits(ref BitsBuffer xBuffer,
}

// Use all the bits but one, see [hac] 14.58 (ii)
int z = LeadingZeros((uint)xh);
int z = BitOperations.LeadingZeroCount((uint)xh);

x = ((xh << 32 + z) | (xm << z) | (xl >> 32 - z)) >> 1;
y = ((yh << 32 + z) | (ym << z) | (yl >> 32 - z)) >> 1;
Expand Down
12 changes: 2 additions & 10 deletions src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,23 +370,15 @@ public static Complex Reciprocal(Complex value)

public override bool Equals([NotNullWhen(true)] object? obj)
{
if (!(obj is Complex)) return false;
return Equals((Complex)obj);
return obj is Complex c && Equals(c);
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
}

public bool Equals(Complex value)
{
return m_real.Equals(value.m_real) && m_imaginary.Equals(value.m_imaginary);
}

public override int GetHashCode()
{
int n1 = 99999997;
int realHash = m_real.GetHashCode() % n1;
int imaginaryHash = m_imaginary.GetHashCode();
int finalHash = realHash ^ imaginaryHash;
return finalHash;
}
public override int GetHashCode() => HashCode.Combine(m_real, m_imaginary);

public override string ToString() => $"({m_real}, {m_imaginary})";

Expand Down
89 changes: 14 additions & 75 deletions src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,20 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.InteropServices;

namespace System.Numerics
{
[StructLayout(LayoutKind.Explicit)]
internal struct DoubleUlong
{
[FieldOffset(0)]
public double dbl;
[FieldOffset(0)]
public ulong uu;
}

internal static class NumericsHelpers
{
private const int kcbitUint = 32;

public static void GetDoubleParts(double dbl, out int sign, out int exp, out ulong man, out bool fFinite)
{
DoubleUlong du;
du.uu = 0;
du.dbl = dbl;
ulong bits = BitConverter.DoubleToUInt64Bits(dbl);

sign = 1 - ((int)(du.uu >> 62) & 2);
man = du.uu & 0x000FFFFFFFFFFFFF;
exp = (int)(du.uu >> 52) & 0x7FF;
sign = 1 - ((int)(bits >> 62) & 2);
pgovind marked this conversation as resolved.
Show resolved Hide resolved
man = bits & 0x000FFFFFFFFFFFFF;
exp = (int)(bits >> 52) & 0x7FF;
if (exp == 0)
{
// Denormalized number.
Expand All @@ -51,15 +39,14 @@ public static void GetDoubleParts(double dbl, out int sign, out int exp, out ulo

public static double GetDoubleFromParts(int sign, int exp, ulong man)
{
DoubleUlong du;
du.dbl = 0;
ulong bits;

if (man == 0)
du.uu = 0;
bits = 0;
else
{
// Normalize so that 0x0010 0000 0000 0000 is the highest bit set.
int cbitShift = CbitHighZero(man) - 11;
int cbitShift = BitOperations.LeadingZeroCount(man) - 11;
if (cbitShift < 0)
man >>= -cbitShift;
else
Expand All @@ -74,7 +61,7 @@ public static double GetDoubleFromParts(int sign, int exp, ulong man)
if (exp >= 0x7FF)
{
// Infinity.
du.uu = 0x7FF0000000000000;
bits = 0x7FF0000000000000;
}
else if (exp <= 0)
{
Expand All @@ -83,25 +70,25 @@ public static double GetDoubleFromParts(int sign, int exp, ulong man)
if (exp < -52)
{
// Underflow to zero.
du.uu = 0;
bits = 0;
}
else
{
du.uu = man >> -exp;
Debug.Assert(du.uu != 0);
bits = man >> -exp;
Debug.Assert(bits != 0);
}
}
else
{
// Mask off the implicit high bit.
du.uu = (man & 0x000FFFFFFFFFFFFF) | ((ulong)exp << 52);
bits = (man & 0x000FFFFFFFFFFFFF) | ((ulong)exp << 52);
}
}

if (sign < 0)
du.uu |= 0x8000000000000000;
bits |= 0x8000000000000000;

return du.dbl;
return BitConverter.UInt64BitsToDouble(bits);
}

// Do an in-place two's complement. "Dangerous" because it causes
Expand Down Expand Up @@ -139,53 +126,5 @@ public static uint Abs(int a)
return ((uint)a ^ mask) - mask;
}
}

public static uint CombineHash(uint u1, uint u2)
{
return ((u1 << 7) | (u1 >> 25)) ^ u2;
}

public static int CombineHash(int n1, int n2)
{
return unchecked((int)CombineHash((uint)n1, (uint)n2));
}

public static int CbitHighZero(uint u)
{
if (u == 0)
return 32;

int cbit = 0;
if ((u & 0xFFFF0000) == 0)
{
cbit += 16;
u <<= 16;
}
if ((u & 0xFF000000) == 0)
{
cbit += 8;
u <<= 8;
}
if ((u & 0xF0000000) == 0)
{
cbit += 4;
u <<= 4;
}
if ((u & 0xC0000000) == 0)
{
cbit += 2;
u <<= 2;
}
if ((u & 0x80000000) == 0)
cbit += 1;
return cbit;
}

public static int CbitHighZero(ulong uu)
{
if ((uu & 0xFFFFFFFF00000000) == 0)
return 32 + CbitHighZero((uint)uu);
return CbitHighZero((uint)(uu >> 32));
}
}
}