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

Improve CopySign performance for integer types #90970

Closed
wants to merge 10 commits into from
Closed

Improve CopySign performance for integer types #90970

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
271014a
Fix #77579
LEI-Hongfaan Aug 23, 2023
630c704
Merge branch 'main' of https://github.com/LEI-Hongfaan/dotnet-runtime
LEI-Hongfaan Aug 23, 2023
e8cee13
Code style: Put constants on the right side.
LEI-Hongfaan Aug 23, 2023
1dd6e3d
Use Math.ThrowNegateTwosCompOverflow() to throw OverflowException.
LEI-Hongfaan Aug 23, 2023
12274ee
Use a better bit operation for SignZeroToOne.
LEI-Hongfaan Aug 24, 2023
725bd61
Code Style: Remove some unnecessary `unchecked`
LEI-Hongfaan Aug 24, 2023
fe5b230
Avoid expensive imul in Int128.CopySign for 32-bit
LEI-Hongfaan Aug 29, 2023
64044bd
Update src/libraries/System.Private.CoreLib/src/System/Int64.cs
LEI-Hongfaan Nov 8, 2023
3e650ba
Rename SignZeroToOne to SignOrOneIfZero.
LEI-Hongfaan Nov 8, 2023
3c1be6d
Merge branch 'main' of https://github.com/LEI-Hongfaan/dotnet-runtime
LEI-Hongfaan Nov 8, 2023
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
18 changes: 4 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/Int128.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1290,22 +1290,12 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static Int128 CopySign(Int128 value, Int128 sign)
{
Int128 absValue = value;

if (IsNegative(absValue))
{
absValue = -absValue;
}

if (IsPositive(sign))
// Int128.MinValue != value || 0 > sign
if (long.MinValue != unchecked((long)value._upper) || 0 != value._lower || Int128.IsNegative(sign))
{
if (IsNegative(absValue))
{
Math.ThrowNegateTwosCompOverflow();
}
return absValue;
return value * Math.SignZeroToOne(unchecked((long)value._upper ^ (long)sign._upper));
}
return -absValue;
return checked(-unchecked((long)value._upper)); // throws an OverflowException
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down
19 changes: 3 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/Int16.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,24 +631,11 @@ public static short Log2(short value)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static short CopySign(short value, short sign)
{
short absValue = value;

if (absValue < 0)
{
absValue = (short)(-absValue);
}

if (sign >= 0)
if (short.MinValue != value || 0 > sign)
LEI-Hongfaan marked this conversation as resolved.
Show resolved Hide resolved
{
if (absValue < 0)
{
Math.ThrowNegateTwosCompOverflow();
}

return absValue;
return unchecked((short)(value * Math.SignZeroToOne(value ^ sign)));
}

return (short)(-absValue);
return checked((short)unchecked(-value)); // throws an OverflowException
LEI-Hongfaan marked this conversation as resolved.
Show resolved Hide resolved
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down
19 changes: 3 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/Int32.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,24 +666,11 @@ public static int Log2(int value)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static int CopySign(int value, int sign)
{
int absValue = value;

if (absValue < 0)
{
absValue = -absValue;
}

if (sign >= 0)
if (int.MinValue != value || 0 > sign)
{
if (absValue < 0)
{
Math.ThrowNegateTwosCompOverflow();
}

return absValue;
return value * Math.SignZeroToOne(value ^ sign);
}

return -absValue;
return checked(-value); // throws an OverflowException
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down
19 changes: 3 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/Int64.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,24 +663,11 @@ public static long Log2(long value)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static long CopySign(long value, long sign)
LEI-Hongfaan marked this conversation as resolved.
Show resolved Hide resolved
{
long absValue = value;

if (absValue < 0)
{
absValue = -absValue;
}

if (sign >= 0)
if (long.MinValue != value || 0 > sign)
{
if (absValue < 0)
{
Math.ThrowNegateTwosCompOverflow();
}

return absValue;
return value * Math.SignZeroToOne(value ^ sign);
}

return -absValue;
return checked(-value); // throws an OverflowException
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down
19 changes: 3 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/IntPtr.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,24 +674,11 @@ public static nint Log2(nint value)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static nint CopySign(nint value, nint sign)
{
nint absValue = value;

if (absValue < 0)
{
absValue = -absValue;
}

if (sign >= 0)
if (nint.MinValue != value || 0 > sign)
{
if (absValue < 0)
{
Math.ThrowNegateTwosCompOverflow();
}

return absValue;
return value * Math.SignZeroToOne(value ^ sign);
}

return -absValue;
return checked(-value); // throws an OverflowException
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down
21 changes: 21 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Math.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,27 @@ public static int Sign(float value)
throw new ArithmeticException(SR.Arithmetic_NaN);
}

// Helper functions for CopySign
// >= 0: returns 1
// < 0: returns -1
[MethodImpl(MethodImplOptions.AggressiveInlining)]
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
internal static int SignZeroToOne(int value)
Copy link
Member

Choose a reason for hiding this comment

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

This name isn't "clear" IMO and needs something that makes its meaning clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SignOrOneIfZero?

{
return unchecked((value >> (32 - 1)) - (~value >> (32 - 1)));
LEI-Hongfaan marked this conversation as resolved.
Show resolved Hide resolved
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
internal static int SignZeroToOne(nint value)
{
return unchecked((int)(value >> (8 * nint.Size - 1)) - (int)(~value >> (8 * nint.Size - 1)));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
internal static int SignZeroToOne(long value)
{
return unchecked((int)(value >> (64 - 1)) - (int)(~value >> (64 - 1)));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static decimal Truncate(decimal d)
{
Expand Down
19 changes: 3 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/SByte.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,24 +592,11 @@ public static sbyte Log2(sbyte value)
/// <inheritdoc cref="INumber{TSelf}.CopySign(TSelf, TSelf)" />
public static sbyte CopySign(sbyte value, sbyte sign)
{
sbyte absValue = value;

if (absValue < 0)
{
absValue = (sbyte)(-absValue);
}

if (sign >= 0)
if (sbyte.MinValue != value || 0 > sign)
{
if (absValue < 0)
{
Math.ThrowNegateTwosCompOverflow();
}

return absValue;
return unchecked((sbyte)(value * Math.SignZeroToOne(value ^ sign)));
}

return (sbyte)(-absValue);
return checked((sbyte)unchecked(-value)); // throws an OverflowException
}

/// <inheritdoc cref="INumber{TSelf}.Max(TSelf, TSelf)" />
Expand Down