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

Simplest CUDA half type conflict resolution implementation. #27

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

oxt3479
Copy link
Contributor

@oxt3479 oxt3479 commented Jul 17, 2020

This implementation will simply place half into a namespace if compiling for the cuda kernel to prevent conflicts with the identically named cuda half type.

Note: A few definitions needed to be moved around to avoid placing functions within the namespace and breaking existing testing. (operator <<, >>, and printBits)

Issues moving forward:

1: Testing of the cuda::half type's behaviour can be added, however the cuda half type is more picky about comparison and type conversion. For instance:

half h1;
...
assert (h1 == 1);

Will produce compiler errors if written with the cuda half (but not for imath::half). It must instead be written this way:

half h1;
...
assert(float(h1) == 1);

2: The cuda half type also can not be created on host code and therefore printing using cout will not be trivial. Most tests use this heavily.

The bottom line: making the cuda::half behave just as the imath::half type requires considerable reworking and does not seem to be fully attainable for many reasons.
Certainly testing can be done it will just not be the same as the testing already implemented under HalfTest/

Signed-off-by: Owen Thompson <oxt3479@rit.edu>

#ifndef __CUDACC__
using half = IMATH_INTERNAL_NAMESPACE::half;
#else
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation

@oxt3479 oxt3479 merged commit 5386ee0 into AcademySoftwareFoundation:master Jul 21, 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.

3 participants