- Notifications
You must be signed in to change notification settings - Fork5.1k
Adding vectorized implementations of Exp to Vector64/128/256/512#97114
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedJan 17, 2024
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, 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. |
ghost commentedJan 17, 2024
Tagging subscribers to this area: @dotnet/area-system-numerics Issue DetailsThis makes progress towards#93513
|
| private const float V_EXPF_MIN = -103.97208f; | ||
| private const float V_EXPF_MAX = 88.72284f; | ||
| private const float V_EXPF_MAX =+88.72284f; |
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.
This is just style, right?
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.
Correct, just better aligning the numbers, which I personally find more visually appealing/easier to read.
| @@ -1426,6 +1426,50 @@ public static bool EqualsAny<T>(Vector128<T> left, Vector128<T> right) | |||
| || Vector64.EqualsAny(left._upper, right._upper); | |||
| } | |||
| internal static Vector128<T> Exp<T>(Vector128<T> vector) | |||
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.
Is this used somewhere outside of this type, or could it be private?
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.
Copy/paste error. V128/256/512 should be calling the next smaller size on the upper/lower halves, as that gives the simplest implementation and tends to better performance on hardware that can't directly accelerate that size.
Log/Log2 are doing the right thing, just messed up here forExp
| @@ -578,19 +814,19 @@ internal static class VectorMath | |||
| if (typeof(TVectorInt64) == typeof(Vector64<long>)) | |||
| { | |||
| return (TVectorDouble)(object)Vector64.ConvertToDouble((Vector64<long>)(object)vector); | |||
| result = (TVectorDouble)(object)Vector64.ConvertToDouble((Vector64<long>)(object)vector); | |||
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.
What drove the changes here around how the result is returned?
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.
Consistency around the intent/way the code was written.
Most compilers tend to perform better and need to do less work with a single return path, the JIT has historically been no exception, and since we need to handle returning in thethrow path anyways, I had intended for it to be setup like this originally.
…net#97114)* Adding vectorized implementations of Exp to Vector64/128/256/512* Accelerate TensorPrimitives.Exp for double* Ensure the right allowedVariance is used for the vectorized exp tests* Ensure V128/256/512 defers to the next smaller vector size by operating on the lower/upper halves* Ensure the right allowedVariance amounts are used for the vectorized Exp(float) tests* Ensure we call Exp and that the methods are properly inlined* Skip the Exp test for Vector128/256/512 on Mono due todotnet#97176
This makes progress towards#93513