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

Added CUDA __host__ __device__ with macro to Vector, Matrix, Limits, … #38

Merged
merged 3 commits into from
Aug 10, 2020

Conversation

oxt3479
Copy link
Contributor

@oxt3479 oxt3479 commented Aug 6, 2020

These changes are verified to compile with CUDA compiler version 11.0 (and should work on earlier versions). All changes made in this PR are in service of facilitating compilation.

Most of the changes are added macros but there are some additional changes that were necessary.
1:

Functions within matrix functions like: Math::sqrt(x) and Math::sin(x)
were replaced with: sqrt((T) x) and sin((T) x)

This allows for the CUDA compiler to use device versions of these functions when it is necessary. If this were not the case then the function that calls sqrt/sin/cos (for instance length()) could not be executed on the device

2:

Because CUDA does not allow for exceptions there are changes to: invert, inverse, gjInvert, and gjInverse for M22 M33 and M44

There are now two versions of each of these functions now, one which takes a bool singExc to specify whether it can throw an exception and one which takes in no parameters and never throws. The original implementation defaulted singExc to false. Therefore, the behaviour of any existing code should not change, it may in fact run slightly faster as it does not need to check for singExc if there is no bool passed in.

These changes are focused on Vector and Matrix operations. Changes to other files were necessary as Vector and Matrix operations call some of them (for instance those in ImathLimits.h)

…and msc needed functions to compile with CUDA

Signed-off-by: Owen Thompson <[email protected]>
Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

Looking good. Could you add duplicate unit tests for the exception-less new functions please? It's important that the two versions don't accidentally deviate.

constexpr static T baseTypeMax() { return limits<T>::max(); }
constexpr static T baseTypeSmallest() { return limits<T>::smallest(); }
constexpr static T baseTypeEpsilon() { return limits<T>::epsilon(); }
IMATH_HOSTDEVICE constexpr static T baseTypeMin() { return limits<T>::min(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't remember why these constexpr haven't been replaced by the IMATH_CONSTEXPR14 macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it has not been replaced then it means that it is capable of being compiled as constexr under the c++11 standard. If they were to be replaced with IMATH_CONSTEXR14 then it would not be constexr for c++11, but can be for c++14.

These functions are only a return statement which allows for c++11 to compile them as constexpr.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh! Thanks for the explanation!

Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

Very exciting, I've wanted this for a long time!

@oxt3479 oxt3479 merged commit 1ffffbc into AcademySoftwareFoundation:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants