Skip to content

Simplify/optimize Int128 comparison operators #101848

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

Merged
merged 4 commits into from
May 9, 2024
Merged

Simplify/optimize Int128 comparison operators #101848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70e457c
Optimize In128 comparison operators
lilinus Apr 27, 2024
848c12b
Merge branch 'main' into optimize-int128-comparisons
lilinus May 3, 2024
75a3579
Document how Int128 comparison operators work
lilinus May 5, 2024
0630152
Merge branch 'main' into optimize-int128-comparisons
lilinus May 5, 2024
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
54 changes: 18 additions & 36 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 @@ -1043,57 +1043,39 @@ public static Int128 Log2(Int128 value)
/// <inheritdoc cref="IComparisonOperators{TSelf, TOther, TResult}.op_LessThan(TSelf, TOther)" />
public static bool operator <(Int128 left, Int128 right)
{
if (IsNegative(left) == IsNegative(right))
{
return (left._upper < right._upper)
|| ((left._upper == right._upper) && (left._lower < right._lower));
}
else
{
return IsNegative(left);
}
// If left and right have different signs: Signed comparison of _upper gives result since it is stored as two's complement
// If signs are equal and left._upper < right._upper: left < right for negative and positive values,
// since _upper is upper 64 bits in two's complement.
// If signs are equal and left._upper > right._upper: left > right for negative and positive values,
// since _upper is upper 64 bits in two's complement.
// If left._upper == right._upper: unsigned comparison of _lower gives the result for both negative and positive values since
// lower values are lower 64 bits in two's complement.
return ((long)left._upper < (long)right._upper)
Copy link
Member

@tannergooding tannergooding May 3, 2024

Choose a reason for hiding this comment

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

A comment (in the code) explaining why this is valid would be beneficial.

Actually brute forcing this for some struct S16 { byte _lower; byte _upper; } is pretty trivial to do, but so is describing why this works out given the two's complement representation. -- Notably this comes down to the fact that all negative values have the sign bit set, so if lhs.upper < rhs.upper in signed form, then either lhs/rhs have different signs, and the single comparison was sufficient or they have the same sign and the same was true.

There may be other optimizations available here as well... In particular, comparisons x cmp y in general are done as if x - y and then setting CPU flags, without actually producing the result. That general premise is why the "most optimal" pattern is actually cmp lower; sbb upper; setcc

Copy link
Contributor Author

@lilinus lilinus May 5, 2024

Choose a reason for hiding this comment

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

I tried my best documenting it with comments.

|| ((left._upper == right._upper) && (left._lower < right._lower));
}

/// <inheritdoc cref="IComparisonOperators{TSelf, TOther, TResult}.op_LessThanOrEqual(TSelf, TOther)" />
public static bool operator <=(Int128 left, Int128 right)
{
if (IsNegative(left) == IsNegative(right))
{
return (left._upper < right._upper)
|| ((left._upper == right._upper) && (left._lower <= right._lower));
}
else
{
return IsNegative(left);
}
// See comment in < operator for how this works.
return ((long)left._upper < (long)right._upper)
|| ((left._upper == right._upper) && (left._lower <= right._lower));
}

/// <inheritdoc cref="IComparisonOperators{TSelf, TOther, TResult}.op_GreaterThan(TSelf, TOther)" />
public static bool operator >(Int128 left, Int128 right)
{
if (IsNegative(left) == IsNegative(right))
{
return (left._upper > right._upper)
|| ((left._upper == right._upper) && (left._lower > right._lower));
}
else
{
return IsNegative(right);
}
// See comment in < operator for how this works.
return ((long)left._upper > (long)right._upper)
|| ((left._upper == right._upper) && (left._lower > right._lower));
}

/// <inheritdoc cref="IComparisonOperators{TSelf, TOther, TResult}.op_GreaterThanOrEqual(TSelf, TOther)" />
public static bool operator >=(Int128 left, Int128 right)
{
if (IsNegative(left) == IsNegative(right))
{
return (left._upper > right._upper)
|| ((left._upper == right._upper) && (left._lower >= right._lower));
}
else
{
return IsNegative(right);
}
// See comment in < operator for how this works.
return ((long)left._upper > (long)right._upper)
|| ((left._upper == right._upper) && (left._lower >= right._lower));
}

//
Expand Down
24 changes: 24 additions & 0 deletions src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Int128Tests.GenericMath.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,12 @@ public static void op_GreaterThanTest()
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(MaxValue, 1));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(MinValue, 1));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(NegativeOne, 1));

Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(Zero, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(One, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(MaxValue, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(MinValue, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThan(NegativeOne, NegativeOne));
}

[Fact]
Expand All @@ -1022,6 +1028,12 @@ public static void op_GreaterThanOrEqualTest()
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(MaxValue, 1));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(MinValue, 1));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(NegativeOne, 1));

Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(Zero, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(One, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(MaxValue, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(MinValue, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_GreaterThanOrEqual(NegativeOne, NegativeOne));
}

[Fact]
Expand All @@ -1032,6 +1044,12 @@ public static void op_LessThanTest()
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(MaxValue, 1));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(MinValue, 1));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(NegativeOne, 1));

Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(Zero, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(One, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(MaxValue, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(MinValue, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThan(NegativeOne, NegativeOne));
}

[Fact]
Expand All @@ -1042,6 +1060,12 @@ public static void op_LessThanOrEqualTest()
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(MaxValue, 1));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(MinValue, 1));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(NegativeOne, 1));

Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(Zero, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(One, NegativeOne));
Assert.False(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(MaxValue, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(MinValue, NegativeOne));
Assert.True(ComparisonOperatorsHelper<Int128, Int128, bool>.op_LessThanOrEqual(NegativeOne, NegativeOne));
}

//
Expand Down