Skip to content

Conversation

jszuppe
Copy link
Contributor

@jszuppe jszuppe commented Sep 20, 2016

I added a few missing lambda wrappers for OpenCL builtin function (see #659), but I noticed that a lot more is missing. @kylelutz, did you skip them on purpose or should I add all of them?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 83.342% when pulling 90d106e on haahh:pr_lambda_funcs into 8621106 on boostorg:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 83.346% when pulling b3d8e49 on haahh:pr_lambda_funcs into 8621106 on boostorg:develop.

BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(abs_diff)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(add_sat)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(hadd)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(hradd)
Copy link
Contributor

Choose a reason for hiding this comment

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

6.13.3 Integer functions, 4th function is:rhadd not hradd

BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(exp)
BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(exp2)
BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(exp10)
BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(fabs)
Copy link
Contributor

Choose a reason for hiding this comment

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

6.13.2 Math functions, expm1 is missing

BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(fmax)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(fmin)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(fmod)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(hypot)
Copy link
Contributor

@kenba kenba Sep 23, 2016

Choose a reason for hiding this comment

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

6.13.2 Math functions: fract and frexp are missing

BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(log1p)
BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(logb)
BOOST_COMPUTE_LAMBDA_WRAP_TERNARY_FUNCTION(mad)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(nextafter)
Copy link
Contributor

Choose a reason for hiding this comment

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

6.13.2 Math functions: maxmag, minmag, modf & nan are missing.

BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(pown)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(powr)
BOOST_COMPUTE_LAMBDA_WRAP_BINARY_FUNCTION(remainder)
BOOST_COMPUTE_LAMBDA_WRAP_UNARY_FUNCTION_T(rint)
Copy link
Contributor

Choose a reason for hiding this comment

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

6.13.2 Math function: remquo is missing

Copy link
Contributor

@kenba kenba left a comment

Choose a reason for hiding this comment

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

Excellent work Jakub. It was a much bigger change than I was expecting. I particularly liked that grouped and ordered the functions as in the OpenCL spec, it made reviewing much easier.

I only reviewed the changes for completeness against the OpenCL language spec. I've put comments where I think that functions are missing.

Also what about the optional half_* and native_* functions. Should they also be included?

@jszuppe
Copy link
Contributor Author

jszuppe commented Sep 23, 2016

Thanks for the comments, I'll look at them later today.

@jszuppe
Copy link
Contributor Author

jszuppe commented Sep 24, 2016

I just need to add nan and everything will be ready.

/// vector_type_producer<int_, 1>::type == int_
/// \endcode
template<class Scalar, size_t N>
struct vector_type_producer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this the same as make_vector_type<T, N> from <boost/compute/type_traits/make_vector_type.hpp>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes, you're right. I couldn't find it. I'll remove it.

@kylelutz
Copy link
Collaborator

Thanks for adding the rest of these! Yeah, I don't think I skipped them on purpose, just never got around to adding them.

@jszuppe
Copy link
Contributor Author

jszuppe commented Sep 24, 2016

I'll add nan, and maybe those native_ and half_ functions later today and then this pull request will be ready.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 83.351% when pulling 0705750 on haahh:pr_lambda_funcs into 8621106 on boostorg:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 83.351% when pulling 0705750 on haahh:pr_lambda_funcs into 8621106 on boostorg:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 83.351% when pulling e25bb7d on haahh:pr_lambda_funcs into 8621106 on boostorg:develop.

@kylelutz kylelutz merged commit a94c75d into boostorg:develop Oct 19, 2016
@kylelutz
Copy link
Collaborator

Merged! Thanks for adding all these!

@kenba
Copy link
Contributor

kenba commented Oct 20, 2016

I've just tested the merged changes in the develop branch against the original failing code for issue #659 and I'm delighted to report that it's definitely fixed the issue for normalize and atan2.

Thanks again Jakub.

@jszuppe jszuppe deleted the pr_lambda_funcs branch October 20, 2016 09:02
@jszuppe
Copy link
Contributor Author

jszuppe commented Oct 20, 2016

Glad, I could help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants