[release/7.0] Simplify and fix the Int128 *, /, and % logic (#75567)

* Simplify and fix the Int128 *, /, and % logic

* Apply suggestions from code review

Co-authored-by: tfenise <tfenise@live.com>

* Fixing some code comments

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
Co-authored-by: tfenise <tfenise@live.com>

Author: 3 people

src/libraries/System.Private.CoreLib/src/System/Int128.cs

@@ -1156,7 +1156,7 @@ public static Int128 Log2(Int128 value)
             }
 
             // We simplify the logic here by just doing unsigned division on the
-            // one's complement representation and then taking the correct sign.
+            // two's complement representation and then taking the correct sign.
 
             ulong sign = (left._upper ^ right._upper) & (1UL << 63);
 
@@ -1172,17 +1172,13 @@ public static Int128 Log2(Int128 value)
 
             UInt128 result = (UInt128)(left) / (UInt128)(right);
 
-            if (result == 0U)
-            {
-                sign = 0;
-            }
-            else if (sign != 0)
+            if (sign != 0)
             {
                 result = ~result + 1U;
             }
 
             return new Int128(
-                result.Upper | sign,
+                result.Upper,
                 result.Lower
             );
         }
@@ -1227,36 +1223,8 @@ public static Int128 Log2(Int128 value)
         /// <inheritdoc cref="IModulusOperators{TSelf, TOther, TResult}.op_Modulus(TSelf, TOther)" />
         public static Int128 operator %(Int128 left, Int128 right)
         {
-            // We simplify the logic here by just doing unsigned modulus on the
-            // one's complement representation and then taking the correct sign.
-
-            ulong sign = (left._upper ^ right._upper) & (1UL << 63);
-
-            if (IsNegative(left))
-            {
-                left = ~left + 1U;
-            }
-
-            if (IsNegative(right))
-            {
-                right = ~right + 1U;
-            }
-
-            UInt128 result = (UInt128)(left) % (UInt128)(right);
-
-            if (result == 0U)
-            {
-                sign = 0;
-            }
-            else if (sign != 0)
-            {
-                result = ~result + 1U;
-            }
-
-            return new Int128(
-                result.Upper | sign,
-                result.Lower
-            );
+            Int128 quotient = left / right;
+            return left - (quotient * right);
         }
 
         //
@@ -1273,76 +1241,43 @@ public static Int128 Log2(Int128 value)
         /// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_Multiply(TSelf, TOther)" />
         public static Int128 operator *(Int128 left, Int128 right)
         {
-            // We simplify the logic here by just doing unsigned multiplication on
-            // the one's complement representation and then taking the correct sign.
-
-            ulong sign = (left._upper ^ right._upper) & (1UL << 63);
-
-            if (IsNegative(left))
-            {
-                left = ~left + 1U;
-            }
-
-            if (IsNegative(right))
-            {
-                right = ~right + 1U;
-            }
-
-            UInt128 result = (UInt128)(left) * (UInt128)(right);
-
-            if (result == 0U)
-            {
-                sign = 0;
-            }
-            else if (sign != 0)
-            {
-                result = ~result + 1U;
-            }
-
-            return new Int128(
-                result.Upper | sign,
-                result.Lower
-            );
+            // Multiplication is the same for signed and unsigned provided the "upper" bits aren't needed
+            return (Int128)((UInt128)(left) * (UInt128)(right));
         }
 
         /// <inheritdoc cref="IMultiplyOperators{TSelf, TOther, TResult}.op_CheckedMultiply(TSelf, TOther)" />
         public static Int128 operator checked *(Int128 left, Int128 right)
         {
-            // We simplify the logic here by just doing unsigned multiplication on
-            // the one's complement representation and then taking the correct sign.
-
-            ulong sign = (left._upper ^ right._upper) & (1UL << 63);
-
-            if (IsNegative(left))
-            {
-                left = ~left + 1U;
-            }
+            Int128 upper = BigMul(left, right, out Int128 lower);
 
-            if (IsNegative(right))
+            if (((upper != 0) || (lower < 0)) && ((~upper != 0) || (lower >= 0)))
             {
-                right = ~right + 1U;
-            }
+                // The upper bits can safely be either Zero or AllBitsSet
+                // where the former represents a positive value and the
+                // latter a negative value.
+                //
+                // However, when the upper bits are Zero, we also need to
+                // confirm the lower bits are positive, otherwise we have
+                // a positive value greater than MaxValue and should throw
+                //
+                // Likewise, when the upper bits are AllBitsSet, we also
+                // need to confirm the lower bits are negative, otherwise
+                // we have a large negative value less than MinValue and
+                // should throw.
 
-            UInt128 result = checked((UInt128)(left) * (UInt128)(right));
-
-            if ((long)(result.Upper) < 0)
-            {
                 ThrowHelper.ThrowOverflowException();
             }
 
-            if (result == 0U)
-            {
-                sign = 0;
-            }
-            else if (sign != 0)
-            {
-                result = ~result + 1U;
-            }
+            return lower;
+        }
 
-            return new Int128(
-                result.Upper | sign,
-                result.Lower
-            );
+        internal static Int128 BigMul(Int128 left, Int128 right, out Int128 lower)
+        {
+            // This follows the same logic as is used in `long Math.BigMul(long, long, out long)`
+
+            UInt128 upper = UInt128.BigMul((UInt128)(left), (UInt128)(right), out UInt128 ulower);
+            lower = (Int128)(ulower);
+            return (Int128)(upper) - ((left >> 127) & right) - ((right >> 127) & left);
         }
 
         //

src/libraries/System.Runtime/tests/System/Int128Tests.cs

@@ -458,5 +458,15 @@ public static void TestNegativeNumberParsingWithHyphen()
             CultureInfo ci = CultureInfo.GetCultureInfo("sv-SE");
             Assert.Equal(-15868, Int128.Parse("-15868", NumberStyles.Number, ci));
         }
+
+        [Fact]
+        public static void Runtime75416()
+        {
+            Int128 a = (Int128.MaxValue - 10) * +100;
+            Assert.Equal(a, -1100);
+
+            Int128 b = (Int128.MaxValue - 10) * -100;
+            Assert.Equal(b, +1100);
+        }
     }
 }

src/libraries/System.Runtime/tests/System/UInt128Tests.cs

@@ -441,5 +441,15 @@ public static void TryFormat(UInt128 i, string format, IFormatProvider provider,
                 Assert.Equal(expected.ToLowerInvariant(), new string(actual));
             }
         }
+
+        [Fact]
+        public static void Runtime75416()
+        {
+            UInt128 a = (UInt128Tests_GenericMath.Int128MaxValue - 10u) * +100u;
+            Assert.Equal(a, (UInt128)(Int128)(-1100));
+
+            UInt128 b = (UInt128Tests_GenericMath.Int128MaxValue - 10u) * (UInt128)(Int128)(-100);
+            Assert.Equal(b, 1100u);
+        }
     }
 }