Re: [boost] [compute] review

On Sun, Dec 28, 2014 at 10:41 PM, Kyle Lutz wrote:

On Sun, Dec 28, 2014 at 1:54 AM, Gruenke,Matt wrote:

...

I'd agree with Andrey Semashev, regarding thread safety, except I think it shouldn't be an option at all. That said, not all operations on all objects need be thread safe. I think the Boost.ASIO documentation provides a good example of how to express different thread safety limitations. Now, I haven't examined the use of thread-local storage, but do you feel there's a strong case to be made for the thread safety it's providing (especially, given that you seem to feel it's non-essential)?

I absolutely do not feel it is non-essential. The reason for it currently being a compile-time configurable option is that enabling it on non-C++11 compilers would make the library non-header-only (it would require the users to link to Boost.Thread).

I'd say, in C++03 it is only nominally header only. I consider the multi-threaded mode as the default (i.e. it is what I want in 99% of use cases and I think that's common), and in this mode users will have to link with Boost.Thread anyway. If you truly want to pursue the header-only property, I think you have to make it header-only in all typical configs.

...

I'm a bit concerned about the use of type names ending in '_', such as float4_. Is this consistent with Boost conventions? I've only seen that used in other Boost libraries to denote class member variables.

I'm not sure if there are any Boost conventions for/against this (someone please speak up if there are). I chose the trailing underscore for these types as I needed a consistent spelling for representing the fundamental OpenCL types (e.g. "float" or "uint" or "int4") and I couldn't just use the names without the trailing underscore as they'd conflict with the C++ reserved keywords (e.g. "float", "int"). Using a leading underscore (e.g. "_float4") looked worse to me, so I used a trailing underscore. But I'd definitely be open to hearing other ideas.

There is precedent of the standard int types (e.g. uint32_t), but I assume in your case int4_ means a vector of 4 ints, right? I'd say, a better name would reflect the fact that this is a vector type and also specify the element type. I don't know if OpenCL defines sizes of its types - if it does, so should Boost.Compute. A few examples of what I have in mind: v4int_t, v4uint32_t, uint32x4_t, floatx4_t. You may also want to see how Boost.SIMD deals with vector types (packs, in its terminology).