-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Adding the convenience Avx.Compare* overloads #34100
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
I found #34094 while implementing these APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with just one question.
src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/X86/Avx.cs
Show resolved
Hide resolved
/// CMPPS ymm, ymm/m256, imm8(0) | ||
/// The above native signature does not exist. We provide this additional overload for completeness. | ||
/// </summary> | ||
public static Vector256<float> CompareEqual(Vector256<float> left, Vector256<float> right) => Compare(left, right, FloatComparisonMode.OrderedEqualNonSignaling); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tannergooding Can you explain why CompareEqual corresponds to OrderedEqualNonSignaling while CompareGreaterThan corresponds to OrderedGreaterThanSignaling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simplest explanation is because that is what the immediate values available to Sse/Sse2 are (imm8 == 0x00 through 0x07, inclusive; 0x08 through 0x1F are Avx+):
The lengthier explanation is that Signaling
when floating-point exceptions are supported (we don't support and explicitly disable them in .NET) throw for SNaN
inputs but not for QNaN
. The comparison operations like ==
, !=
, >
, >=
, <
, and <=
don't expect NaN
as inputs and so they signal by default; while the Ordered
/Unordered
comparisons do expect NaN
inputs and so they don't.
This resolves #31193 by implementing the approved
Avx.Compare*
convenience overloads. There is no JIT side work for these as they just forward to the existing implementation that takes theFloatComparisonMode
enumCC. @CarolEidt, @echesakovMSFT