[vlc-devel] [RFC] Atomic API

[Rafaël Carré]

Hi,

Le 05/12/2013 22:47, Felix Abecassis a écrit :
> Hi,
> 
> In order to compile VLC with MSVC I recently implemented atomic operations
> in vlc_atomic.h using the Interlocked API
> (http://msdn.microsoft.com/en-us/library/windows/desktop/ms686360%28v=vs.85%29.aspx#interlocked_functions<(http://msdn.microsoft.com/en-us/library/sbhbke0y%28v=vs.110%29.aspx>
> ).
> 
> However when compiling for WinRT we discovered some operations are not
> implemented for all types, for instance InterlockedExchange8 is only
> available for desktop apps. This mean we cannot currently provide an
> implementation of atomic load/store operations on char/bool types. Since
> some important files (stream_demux.c, picture_pool.c, variables.h and
> win32/thread.c) are using the atomic_bool type, we need a way to solve this
> problem.
> 
> I've found three options and I would like your opinion in order to decide
> which one is the best.

Thanks for the options

> 1) Fill the holes in the Interlocked API by implementing the missing
> functions. We could use locks similarly to what is currently being
> implemented in vlc_atomic.h when no builtin atomic operations are used.

This one is fast to do but not really efficient.

> 2) Replace the aforementioned occurrences of variables of type atomic_bool
> and use the type vlc_atomic_t instead. Some modules take this approach for
> similar variables. For instance android/thread.c has a variable
> "vlc_atomic_t killed;" whereas win32/thread.c uses "atomic_bool killed;".
> vlc_atomic_t uses the type uintptr_t (4 or 8 bytes).

In fact it is my fault for not using atomic_bool in android/thread.c
The vlc_atomic.h header only declares atomic_bool but does not use it,
contrary to vlc_atomic_t which is used by several functions.

vlc_atomic.h says intel API doesn't support anything below int, so we
might remove atomic_bool/char/short completely and have a consistent API
for different compilers.

> 3) Disable atomic types of size 1 byte for this target. We could force the
> types atomic_bool/atomic_char... to be of a larger size. For instance with
> something like this:
> #if defined (_MSC_VER)
> typedef int atomic_bool;
> #else
> typedef bool atomic_bool;
> #endif
> This approach could also be used if we encounter the same issue with a
> different target.

This works but it could be a bit counter intuitive, even if relying on
the size of a type without using sizeof() should be frowned upon.

> Cheers,

I favor option 2, remove size 1 atomic types (16 bits works with msvc btw?)

Thanks,