Subject: Re: [boost] [compute] review
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2014-12-28 18:38:54
On Sun, Dec 28, 2014 at 10:41 PM, Kyle Lutz <kyle.r.lutz_at_[hidden]> wrote:
> On Sun, Dec 28, 2014 at 1:54 AM, Gruenke,Matt <mgruenke_at_[hidden]> 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).