Intel® oneAPI Threading Building Blocks
Ask questions and share information about adding parallelism to your applications when using this threading library.

Proposed patch 4.3 for simplifying compilation errors for tbb::atomic

Fan_Xp_
Beginner
283 Views

Dear TBB Developers,

Please consider the attached patch for tbb43_20141009oss_src which simplifies the compilation errors for misusing tbb::atomic.

Basically, tbb users sometimes misuse tbb::atomic with unsupported types, for example.
And the compiler-error message about the misuse is not easy to understand since it exposes internal details about tbb.

Here is the compiler-error message of misusing tbb::atomic with a non-supported type with Clang 3.5.

tbb43_20141009oss/include/tbb/atomic.h:220:34: error: implicit instantiation of undefined template 'tbb::internal::aligned_storage<MyPair, 16>'
    aligned_storage<T,sizeof(T)> my_storage;
                                 ^
tbb43_20141009oss/include/tbb/atomic.h:405:16: note: in instantiation of template class 'tbb::internal::atomic_impl<MyPair>' requested here
struct atomic: internal::atomic_impl<T> {
               ^
main.cpp:11:23: note: in instantiation of template class 'tbb::atomic<MyPair>' requested here
  tbb::atomic<MyPair> b;
                      ^
tbb43_20141009oss/include/tbb/atomic.h:93:8: note: template is declared here
struct aligned_storage;

With the attached patch, the compiler-error message shall be like this, with helpful error message.

mytbb43/include/tbb/atomic.h:448:3: error: static_assert failed "tbb::atomic only supports integral type, enumeration type and pointer type"
  __TBB_STATIC_ASSERT(internal::atomic_not_support_type<T>::value, "tbb::atomic only supports integral type, enumeration type and pointer type");
  ^                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mytbb43/include/tbb/tbb_stddef.h:493:44: note: expanded from macro '__TBB_STATIC_ASSERT'
#define __TBB_STATIC_ASSERT(condition,msg) static_assert(condition,msg)
                                           ^
main.cpp:11:23: note: in instantiation of template class 'tbb::atomic<MyPair, tbb::internal::bool_constant<false> >' requested here
  tbb::atomic<MyPair> b;

I think this kind of compiler-error message will be more helpful to tbb users.

Thanks

-xuepeng

0 Kudos
5 Replies
RafSchietekat
Valued Contributor III
283 Views

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

(2015-03-04 Added) Well, "not too large", but also a power of 2, of course...

0 Kudos
Fan_Xp_
Beginner
283 Views

Raf Schietekat wrote:

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

Thanks for your comments, Raf. 

Actually, tbb::atomic does support float and primitive types like structs when they are small. However, the online document here (Type T may be an integral type, enumeration type, or a pointer type) of TBB does not show any clue about supporting float and small structs. Maybe the TBB developers wanna keep this as some "advanced tech", so they didn't document it.

For the proposed patch, it works ok with float and small structs. But I feel it's better to keep the compiler-error message consistency with the official document. And of course, I'm glad to revise the compiler-error message if the developers revise the document.

Thanks

Xuepeng

0 Kudos
Fan_Xp_
Beginner
283 Views

Raf Schietekat wrote:

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

Thanks for your comments, Raf. 

Actually, tbb::atomic does support float and primitive types like structs when they are small. However, the online document here (Type T may be an integral type, enumeration type, or a pointer type) of TBB does not show any clue about supporting float and small structs. Maybe the TBB developers wanna keep this as some "advanced tech", so they didn't document it.

For the proposed patch, it works ok with float and small structs. But I feel it's better to keep the compiler-error message consistency with the official document. And of course, I'm glad to revise the compiler-error message if the developers revise the document.

Thanks

Xuepeng

0 Kudos
RafSchietekat
Valued Contributor III
283 Views

The documentation should be updated.

0 Kudos
Fan_Xp_
Beginner
283 Views

Raf Schietekat wrote:

The documentation should be updated.

I agree with you. Let's see how the TBB developers say.

 

0 Kudos
Reply