Skip to content
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 vectorized implementations of Exp to Vector64/128/256/512 #97114

Merged
merged 7 commits into from
Jan 19, 2024

Conversation

tannergooding
Copy link
Member

This makes progress towards #93513

Copy link

Note regarding the new-api-needs-documentation label:

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
Copy link

ghost commented Jan 17, 2024

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

This makes progress towards #93513

Author: tannergooding
Assignees: tannergooding
Labels:

area-System.Numerics, new-api-needs-documentation

Milestone: -


private const float V_EXPF_MIN = -103.97208f;
private const float V_EXPF_MAX = 88.72284f;
private const float V_EXPF_MAX = +88.72284f;
Copy link
Member

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?

Copy link
Member Author

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)
Copy link
Member

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?

Copy link
Member Author

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 for Exp

@@ -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);
Copy link
Member

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?

Copy link
Member Author

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 the throw path anyways, I had intended for it to be setup like this originally.

@tannergooding tannergooding merged commit c53d221 into dotnet:main Jan 19, 2024
174 of 178 checks passed
@tannergooding tannergooding deleted the fix-93513 branch January 19, 2024 04:37
tmds pushed a commit to tmds/runtime that referenced this pull request Jan 23, 2024
…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 to dotnet#97176
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants