it's not so crucial to have atm, but nice to solve it still. We can reformulate out usage of atomic_cas and make it return non-zero if value changed, which would simply mean we need to update non-OSX versions?
Other thing is that update_maximum() from guardedalloc is not really atomic as well, that would also need to be addressed. On Wed, Jul 1, 2015 at 12:39 PM, Bastien Montagne <[email protected]> wrote: > Ah yes indeed, thanks! > > Also, now in the 'cas' case on OSX, we have this code: > > uint64_t init_val = *v; > OSAtomicCompareAndSwap64((int64_t)old, (int64_t)_new, (int64_t *)v); > return init_val; > > (because OSX has no primitive returning original value of 'v'). > > As far as I can see, this operation is no more strictly atomic: > * thread A sets its init_val > * thread B calls OSAtomicCompareAndSwap64() and change value of v > * thread A calls OSAtomicCompareAndSwap64() based on 'new' v value > * thread A returns invalid orginal value... > > Probably not a crucial issue currently? > > > Le 01/07/2015 11:36, Brecht Van Lommel a écrit : > > "InterlockedExchangeAdd(p, x) + x" works exactly the same as" > > __sync_add_and_fetch(p, x)", no need to use asm. > > > > On Wed, Jul 1, 2015 at 9:53 AM, Bastien Montagne <[email protected]> > wrote: > >> So, trying to hunt an strange issue with Windows scons builds of > >> filebrowser work, I found that our attomic operations (defined in > >> intern/atomic/atomci_ops.h) do not return the same things on MSVC than > >> with other OS/compilers. > >> > >> With GCC & co we use `__sync_add_and_fetch`, on OSX, `OSAtomicAdd64`, > >> with MSVC `InterlockedExchangeAdd64`, and have a fallback implementation > >> in plain assembler. > >> > >> According to documentations (and my limited asm understanding), MSVC > >> version returns the value of `*p` *before* the operation, while all > >> others return it *after* the operation (i.e. return the result of the > >> operation). > >> > >> > https://gcc.gnu.org/onlinedocs/gcc-5.1.0/gcc/_005f_005fsync-Builtins.html > >> > https://msdn.microsoft.com/en-us/library/windows/desktop/ms683599(v=vs.85).aspx > >> > https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/OSAtomicAdd64.3.html > >> > >> I doubt this is desired behavior, and think it could easily generate > >> nasty bug some day… MSVC does not seem to have a 'return value after > >> operation' variant from quick look at the doc? Maybe we should just > >> remove the MSVC case completely and use asm version instead? > >> > >> Bastien > >> _______________________________________________ > >> Bf-committers mailing list > >> [email protected] > >> http://lists.blender.org/mailman/listinfo/bf-committers > > _______________________________________________ > > Bf-committers mailing list > > [email protected] > > http://lists.blender.org/mailman/listinfo/bf-committers > > _______________________________________________ > Bf-committers mailing list > [email protected] > http://lists.blender.org/mailman/listinfo/bf-committers > -- With best regards, Sergey Sharybin _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
