From 271014a24aabadcc1fd2d391ebee8f1ee3f98c0f Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Wed, 23 Aug 2023 02:31:46 +0000 Subject: [PATCH 1/8] Fix #77579 --- .../src/System/Int128.cs | 18 ++++------------ .../src/System/Int16.cs | 19 +++-------------- .../src/System/Int32.cs | 19 +++-------------- .../src/System/Int64.cs | 19 +++-------------- .../src/System/IntPtr.cs | 19 +++-------------- .../System.Private.CoreLib/src/System/Math.cs | 21 +++++++++++++++++++ .../src/System/SByte.cs | 19 +++-------------- 7 files changed, 40 insertions(+), 94 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index 50dce177e1713..e8852cd3d365d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1290,22 +1290,12 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max) /// 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 } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index af144fb860c92..cbe9269b229e6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -631,24 +631,11 @@ public static short Log2(short value) /// 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) { - 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 } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index 2f91c2c2a5970..07978d5d115ef 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -666,24 +666,11 @@ public static int Log2(int value) /// 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 } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index ca170d8c903ed..fa6587b6c77f9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -663,24 +663,11 @@ public static long Log2(long value) /// public static long CopySign(long value, long sign) { - 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 } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index 3dd33c2c66b82..88e8a5cd55ede 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -674,24 +674,11 @@ public static nint Log2(nint value) /// 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 } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Math.cs b/src/libraries/System.Private.CoreLib/src/System/Math.cs index 7d20cc72202dd..4798b0bcf74f6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Math.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Math.cs @@ -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)] + internal static int SignZeroToOne(int value) + { + return unchecked((value >> (32 - 1)) - (~value >> (32 - 1))); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static int SignZeroToOne(nint value) + { + return unchecked((int)(value >> (8 * nint.Size - 1)) - (int)(~value >> (8 * nint.Size - 1))); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + 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) { diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index fb4734d41b69f..cdf6d3f2b5c53 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -592,24 +592,11 @@ public static sbyte Log2(sbyte value) /// 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 } /// From e8cee1346aee632c68ffb9df61e381da9416a75c Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Wed, 23 Aug 2023 16:56:59 +0000 Subject: [PATCH 2/8] Code style: Put constants on the right side. --- src/libraries/System.Private.CoreLib/src/System/Int128.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int32.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/IntPtr.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index e8852cd3d365d..a3eff15a5c97e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1291,7 +1291,7 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max) public static Int128 CopySign(Int128 value, Int128 sign) { // Int128.MinValue != value || 0 > sign - if (long.MinValue != unchecked((long)value._upper) || 0 != value._lower || Int128.IsNegative(sign)) + if (unchecked((long)value._upper) != long.MinValue || value._lower != 0 || Int128.IsNegative(sign)) { return value * Math.SignZeroToOne(unchecked((long)value._upper ^ (long)sign._upper)); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index cbe9269b229e6..5e6db667075b4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -631,7 +631,7 @@ public static short Log2(short value) /// public static short CopySign(short value, short sign) { - if (short.MinValue != value || 0 > sign) + if (value != short.MinValue || sign < 0) { return unchecked((short)(value * Math.SignZeroToOne(value ^ sign))); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index 07978d5d115ef..0316a42ed8634 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -666,7 +666,7 @@ public static int Log2(int value) /// public static int CopySign(int value, int sign) { - if (int.MinValue != value || 0 > sign) + if (value != int.MinValue || sign < 0) { return value * Math.SignZeroToOne(value ^ sign); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index fa6587b6c77f9..d43724612ff6b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -663,7 +663,7 @@ public static long Log2(long value) /// public static long CopySign(long value, long sign) { - if (long.MinValue != value || 0 > sign) + if (value != long.MinValue || sign < 0) { return value * Math.SignZeroToOne(value ^ sign); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index 88e8a5cd55ede..578918b1584d9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -674,7 +674,7 @@ public static nint Log2(nint value) /// public static nint CopySign(nint value, nint sign) { - if (nint.MinValue != value || 0 > sign) + if (value != nint.MinValue || sign < 0) { return value * Math.SignZeroToOne(value ^ sign); } diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index cdf6d3f2b5c53..50320cb4949a6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -592,7 +592,7 @@ public static sbyte Log2(sbyte value) /// public static sbyte CopySign(sbyte value, sbyte sign) { - if (sbyte.MinValue != value || 0 > sign) + if (value != sbyte.MinValue || sign < 0) { return unchecked((sbyte)(value * Math.SignZeroToOne(value ^ sign))); } From 1dd6e3daa171f57ff492f48d1c407903d9d04dbc Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Wed, 23 Aug 2023 19:44:57 +0000 Subject: [PATCH 3/8] Use Math.ThrowNegateTwosCompOverflow() to throw OverflowException. --- src/libraries/System.Private.CoreLib/src/System/Int128.cs | 8 ++++---- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/Int32.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/IntPtr.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index a3eff15a5c97e..31897b69e29eb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1290,12 +1290,12 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max) /// public static Int128 CopySign(Int128 value, Int128 sign) { - // Int128.MinValue != value || 0 > sign - if (unchecked((long)value._upper) != long.MinValue || value._lower != 0 || Int128.IsNegative(sign)) + // Int128.MinValue == value && 0 <= sign + if (unchecked((long)value._upper == long.MinValue && value._lower == 0 && (long)sign._upper >= 0)) { - return value * Math.SignZeroToOne(unchecked((long)value._upper ^ (long)sign._upper)); + Math.ThrowNegateTwosCompOverflow(); } - return checked(-unchecked((long)value._upper)); // throws an OverflowException + return value * Math.SignZeroToOne(unchecked((long)value._upper ^ (long)sign._upper)); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index 5e6db667075b4..c62f2fd0e28cb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -631,11 +631,11 @@ public static short Log2(short value) /// public static short CopySign(short value, short sign) { - if (value != short.MinValue || sign < 0) + if (value == short.MinValue && sign >= 0) { - return unchecked((short)(value * Math.SignZeroToOne(value ^ sign))); + Math.ThrowNegateTwosCompOverflow(); } - return checked((short)unchecked(-value)); // throws an OverflowException + return unchecked((short)(value * Math.SignZeroToOne(value ^ sign))); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index 0316a42ed8634..d1a59c0b4043f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -666,11 +666,11 @@ public static int Log2(int value) /// public static int CopySign(int value, int sign) { - if (value != int.MinValue || sign < 0) + if (value == int.MinValue && sign >= 0) { - return value * Math.SignZeroToOne(value ^ sign); + Math.ThrowNegateTwosCompOverflow(); } - return checked(-value); // throws an OverflowException + return value * Math.SignZeroToOne(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index d43724612ff6b..6c82b876243bf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -663,11 +663,11 @@ public static long Log2(long value) /// public static long CopySign(long value, long sign) { - if (value != long.MinValue || sign < 0) + if (value == long.MinValue && sign >= 0) { - return value * Math.SignZeroToOne(value ^ sign); + Math.ThrowNegateTwosCompOverflow(); } - return checked(-value); // throws an OverflowException + return value * Math.SignZeroToOne(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index 578918b1584d9..4b3548542a492 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -674,11 +674,11 @@ public static nint Log2(nint value) /// public static nint CopySign(nint value, nint sign) { - if (value != nint.MinValue || sign < 0) + if (value == nint.MinValue && sign >= 0) { - return value * Math.SignZeroToOne(value ^ sign); + Math.ThrowNegateTwosCompOverflow(); } - return checked(-value); // throws an OverflowException + return value * Math.SignZeroToOne(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index 50320cb4949a6..727a2b045a3ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -592,11 +592,11 @@ public static sbyte Log2(sbyte value) /// public static sbyte CopySign(sbyte value, sbyte sign) { - if (value != sbyte.MinValue || sign < 0) + if (value == sbyte.MinValue && sign >= 0) { - return unchecked((sbyte)(value * Math.SignZeroToOne(value ^ sign))); + Math.ThrowNegateTwosCompOverflow(); } - return checked((sbyte)unchecked(-value)); // throws an OverflowException + return unchecked((sbyte)(value * Math.SignZeroToOne(value ^ sign))); } /// From 12274eebd73dda4f0299751fcb0cc0d965a059c6 Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Thu, 24 Aug 2023 21:05:33 +0000 Subject: [PATCH 4/8] Use a better bit operation for SignZeroToOne. --- src/libraries/System.Private.CoreLib/src/System/Math.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Math.cs b/src/libraries/System.Private.CoreLib/src/System/Math.cs index 4798b0bcf74f6..316f934afd56b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Math.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Math.cs @@ -1474,19 +1474,19 @@ public static int Sign(float value) [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(int value) { - return unchecked((value >> (32 - 1)) - (~value >> (32 - 1))); + return unchecked((value >> (32 - 2)) | 1); } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(nint value) { - return unchecked((int)(value >> (8 * nint.Size - 1)) - (int)(~value >> (8 * nint.Size - 1))); + return unchecked((int)(value >> (8 * nint.Size - 2)) | 1); } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(long value) { - return unchecked((int)(value >> (64 - 1)) - (int)(~value >> (64 - 1))); + return unchecked((int)(value >> (64 - 2)) | 1); } [MethodImpl(MethodImplOptions.AggressiveInlining)] From 725bd61e606fd655fd70cc87b6e22a6674ce585d Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Thu, 24 Aug 2023 22:22:48 +0000 Subject: [PATCH 5/8] Code Style: Remove some unnecessary `unchecked` --- src/libraries/System.Private.CoreLib/src/System/Int128.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Math.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index 31897b69e29eb..cf36a2c4bda91 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1291,11 +1291,11 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max) public static Int128 CopySign(Int128 value, Int128 sign) { // Int128.MinValue == value && 0 <= sign - if (unchecked((long)value._upper == long.MinValue && value._lower == 0 && (long)sign._upper >= 0)) + if ((long)value._upper == long.MinValue && value._lower == 0 && (long)sign._upper >= 0) { Math.ThrowNegateTwosCompOverflow(); } - return value * Math.SignZeroToOne(unchecked((long)value._upper ^ (long)sign._upper)); + return value * Math.SignZeroToOne((long)value._upper ^ (long)sign._upper); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index c62f2fd0e28cb..80f9e13fa7f16 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -635,7 +635,7 @@ public static short CopySign(short value, short sign) { Math.ThrowNegateTwosCompOverflow(); } - return unchecked((short)(value * Math.SignZeroToOne(value ^ sign))); + return (short)(value * Math.SignZeroToOne(value ^ sign)); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Math.cs b/src/libraries/System.Private.CoreLib/src/System/Math.cs index 316f934afd56b..fcb9099f2d86b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Math.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Math.cs @@ -1474,19 +1474,19 @@ public static int Sign(float value) [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(int value) { - return unchecked((value >> (32 - 2)) | 1); + return (value >> (32 - 2)) | 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(nint value) { - return unchecked((int)(value >> (8 * nint.Size - 2)) | 1); + return (int)(value >> (8 * nint.Size - 2)) | 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static int SignZeroToOne(long value) { - return unchecked((int)(value >> (64 - 2)) | 1); + return (int)(value >> (64 - 2)) | 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index 727a2b045a3ad..177272b29000e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -596,7 +596,7 @@ public static sbyte CopySign(sbyte value, sbyte sign) { Math.ThrowNegateTwosCompOverflow(); } - return unchecked((sbyte)(value * Math.SignZeroToOne(value ^ sign))); + return (sbyte)(value * Math.SignZeroToOne(value ^ sign)); } /// From fe5b2302d225785970faebb5f744e2d3a609d034 Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Tue, 29 Aug 2023 11:48:46 +0000 Subject: [PATCH 6/8] Avoid expensive imul in Int128.CopySign for 32-bit --- .../src/System/Int128.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index cf36a2c4bda91..e4aabecdc342b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1290,12 +1290,33 @@ public static Int128 Clamp(Int128 value, Int128 min, Int128 max) /// public static Int128 CopySign(Int128 value, Int128 sign) { +#if TARGET_32BIT + Int128 t = value; + if (Int128.IsPositive(sign)) + { + if (Int128.IsNegative(t)) + { + t = -t; + if (Int128.IsNegative(t)) + { + Math.ThrowNegateTwosCompOverflow(); + } + } + return t; + } + if (Int128.IsPositive(t)) + { + t = -t; + } + return t; +#else // Int128.MinValue == value && 0 <= sign if ((long)value._upper == long.MinValue && value._lower == 0 && (long)sign._upper >= 0) { Math.ThrowNegateTwosCompOverflow(); } return value * Math.SignZeroToOne((long)value._upper ^ (long)sign._upper); +#endif } /// From 64044bd775608262210a370de92c7b5657657256 Mon Sep 17 00:00:00 2001 From: LEI Hongfaan Date: Wed, 8 Nov 2023 02:56:59 +0000 Subject: [PATCH 7/8] Update src/libraries/System.Private.CoreLib/src/System/Int64.cs Co-authored-by: Adam Sitnik --- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index 6c82b876243bf..f7e36fd9286b4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -661,6 +661,7 @@ public static long Log2(long value) public static long Clamp(long value, long min, long max) => Math.Clamp(value, min, max); /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static long CopySign(long value, long sign) { if (value == long.MinValue && sign >= 0) From 3e650ba92f20eb1983674b42e8317c0117f92149 Mon Sep 17 00:00:00 2001 From: LEI-Hongfaan Date: Wed, 8 Nov 2023 03:02:20 +0000 Subject: [PATCH 8/8] Rename SignZeroToOne to SignOrOneIfZero. --- src/libraries/System.Private.CoreLib/src/System/Int128.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int32.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/IntPtr.cs | 2 +- src/libraries/System.Private.CoreLib/src/System/Math.cs | 6 +++--- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int128.cs b/src/libraries/System.Private.CoreLib/src/System/Int128.cs index e4aabecdc342b..7a6262dcf5be0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int128.cs @@ -1315,7 +1315,7 @@ public static Int128 CopySign(Int128 value, Int128 sign) { Math.ThrowNegateTwosCompOverflow(); } - return value * Math.SignZeroToOne((long)value._upper ^ (long)sign._upper); + return value * Math.SignOrOneIfZero((long)value._upper ^ (long)sign._upper); #endif } diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index 80f9e13fa7f16..24ee8c4a92928 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -635,7 +635,7 @@ public static short CopySign(short value, short sign) { Math.ThrowNegateTwosCompOverflow(); } - return (short)(value * Math.SignZeroToOne(value ^ sign)); + return (short)(value * Math.SignOrOneIfZero(value ^ sign)); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index d1a59c0b4043f..717f4c3762f96 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -670,7 +670,7 @@ public static int CopySign(int value, int sign) { Math.ThrowNegateTwosCompOverflow(); } - return value * Math.SignZeroToOne(value ^ sign); + return value * Math.SignOrOneIfZero(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index 6c82b876243bf..f93c6e28829f0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -667,7 +667,7 @@ public static long CopySign(long value, long sign) { Math.ThrowNegateTwosCompOverflow(); } - return value * Math.SignZeroToOne(value ^ sign); + return value * Math.SignOrOneIfZero(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index 4b3548542a492..0a4c87e5bd899 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -678,7 +678,7 @@ public static nint CopySign(nint value, nint sign) { Math.ThrowNegateTwosCompOverflow(); } - return value * Math.SignZeroToOne(value ^ sign); + return value * Math.SignOrOneIfZero(value ^ sign); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Math.cs b/src/libraries/System.Private.CoreLib/src/System/Math.cs index fcb9099f2d86b..841ecdc2305bd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Math.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Math.cs @@ -1472,19 +1472,19 @@ public static int Sign(float value) // >= 0: returns 1 // < 0: returns -1 [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static int SignZeroToOne(int value) + internal static int SignOrOneIfZero(int value) { return (value >> (32 - 2)) | 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static int SignZeroToOne(nint value) + internal static int SignOrOneIfZero(nint value) { return (int)(value >> (8 * nint.Size - 2)) | 1; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static int SignZeroToOne(long value) + internal static int SignOrOneIfZero(long value) { return (int)(value >> (64 - 2)) | 1; } diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index 177272b29000e..0d61ff474edd2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -596,7 +596,7 @@ public static sbyte CopySign(sbyte value, sbyte sign) { Math.ThrowNegateTwosCompOverflow(); } - return (sbyte)(value * Math.SignZeroToOne(value ^ sign)); + return (sbyte)(value * Math.SignOrOneIfZero(value ^ sign)); } ///