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

Provide conversion functions between cl_half and other types #57

Closed
kpet opened this issue Dec 2, 2019 · 6 comments
Closed

Provide conversion functions between cl_half and other types #57

kpet opened this issue Dec 2, 2019 · 6 comments
Labels
agenda Needs Working Group Discussion

Comments

@kpet
Copy link
Contributor

kpet commented Dec 2, 2019

We currently provide no host-side support for converting between cl_half and other integer or floating-point types.

The following code snippet

cl_float myfloat = ...
cl_half myhalf = (cl_half)myfloat;

results in a floating-point to integer conversion. This is really not nice.

While we can't make the above work in applications written in C, I suggest we provide conversion routines like https://github.com/KhronosGroup/OpenCL-CTS/blob/master/test_common/harness/imageHelpers.cpp#L898 as part of the headers. For applications written in C++, we could make casts work automatically.

@kpet
Copy link
Contributor Author

kpet commented Dec 3, 2019

We should probably also provide isnan, isnormal, isfinite and isinf.

@jrprice
Copy link
Contributor

jrprice commented Dec 19, 2019

+1 to this, and I can do the work to get the initial conversion functions into the headers.

The functions in the CTS make extensive use of the MAKE_HEX_FLOAT macros. They also use fabsf. My current thinking is to rework them a little to avoid depending on these two things (easy enough to perform the comparisons as hex integers). Does this sound OK, or is there a desire to keep the functions exactly as they are in the CTS?

@kpet
Copy link
Contributor Author

kpet commented Dec 20, 2019

That's great! Sounds good to me. I don't think we need/want to keep the functions exactly as they are in the CTS. I thought that we'd actually change the CTS to use the functions provided by the headers. The overall thinking was:

  • starting from the functions in the CTS guarantees that the behaviour of the headers functions match what implementations do today
  • using the functions provided in the headers in the CTS guarantees that applications use what implementations are verified with so at least the behaviour is consistent
  • the last step would be to add unit tests for the headers functions vs. the spec and deal with the outcome of that.

@kpet
Copy link
Contributor Author

kpet commented Aug 5, 2020

@jrprice #60 has done what I originally had in mind here. I've created KhronosGroup/OpenCL-CTS#870 so we don't forget to transition the CTS to these routines. Since you added tests as well under #60 I'm not sure there's anything left to do here other than creating new issues to cover making conversions automatic in C++ programs and consider adding all the is* routines. Did you have something else in mind?

@jrprice
Copy link
Contributor

jrprice commented Aug 5, 2020

Did you have something else in mind?

No, the C++ sugar and is* were the only reasons I didn't close this issue.

@kpet
Copy link
Contributor Author

kpet commented Aug 6, 2020

Thanks! I've created #102 and #103. Closing this issue.

@kpet kpet closed this as completed Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda Needs Working Group Discussion
Projects
None yet
Development

No branches or pull requests

2 participants