[Brl.Blitz] Mac: bbAtomicAdd (etc) deprecated

[GWRon]

While checking for ways to avoid mutexes when simply manipulating integers I saw there is "bbAtomicAdd".

Implementation is:

int bbAtomicAdd( volatile int *p,int incr ){
#if !defined(__ANDROID__) && !defined(_WIN32)
#	ifndef __APPLE__
		return __sync_fetch_and_add(p, incr);
#	else
		return OSAtomicAdd32(incr, p);
#	endif
#else
	return __sync_fetch_and_add(p, incr);
#endif

But OSAtomicAdd32 is deprecated by Apple:

/*! @abstract Atomically adds two 32-bit values.
    @discussion
	This function adds the value given by <code>__theAmount</code> to the
	value in the memory location referenced by <code>__theValue</code>,
 	storing the result back to that memory location atomically.
    @result Returns the new value.
 */
OSATOMIC_DEPRECATED_REPLACE_WITH(atomic_fetch_add)
__OSX_AVAILABLE_STARTING(__MAC_10_4, __IPHONE_2_0)
int32_t	OSAtomicAdd32( int32_t __theAmount, volatile int32_t *__theValue );

I am not sure if we can - and should - already replace it with the suggested atomic_fetch_add

[GWRon]

I already asked this on discord but maybe it fits here too:

in Brl.Treads AtomicSwap() is defined:

Function AtomicSwap:Int( target:Int Var,value:Int )
	Repeat
		Local oldval:Int=target
		If CompareAndSwap( Varptr target,oldval,value ) Return oldval
	Forever
End Function

What happens if a second thread manipulates target after it is stored in Local oldVal but before executing CompareAndSwap ?

Asking as CompareAndSwap is called with "oldVal" rather than "target" (which would use the "original" value then).
This means if something changed "target" meanwhile, the Repeat ... Forever might never exit.

This minimizes the chances I guess.

Function AtomicSwap:Int( target:Int Var,value:Int )
	Repeat
		Local oldval:Int=target
		If CompareAndSwap( Varptr target,target,value ) Return oldval
	Forever
End Function

BUT ... the issue here might remain: If someone really wants to use the returned "oldVal" it might NOT be the one which really was replaced. (this is why there is not just __sync_bool_compare_and_swap (the one used on non-apple) but also __sync_value_compare_and_swap

Also I am not sure what GCC creates out of the C-Code, as the function indirection might lead to more temporary variables and thus ways for other threads to manipulate the "original" meanwhile.

The generated C code for now looks like this:

BBINT brl_threads_CompareAndSwap(BBINT* bbt_target,BBINT bbt_oldValue,BBINT bbt_newValue){
	return bbAtomicCAS((int*)&(*bbt_target),(int)bbt_oldValue,(int)bbt_newValue);
}

BBINT brl_threads_AtomicSwap(BBINT* bbt_target,BBINT bbt_value){
	do{
		BBINT bbt_oldval=*bbt_target;
		if(brl_threads_CompareAndSwap(&(*bbt_target),bbt_oldval,bbt_value)!=0){
			return bbt_oldval;
		}
	}while(!(0));
}

directly calling bbAtomicCas:

Function AtomicSwap:Int( target:Int Var,value:Int )
	Repeat
		Local oldval:Int=target
		If bbAtomicCAS( Varptr target,target,value ) Return oldval
	Forever
End Function

BBINT brl_threads_AtomicSwap(BBINT* bbt_target,BBINT bbt_value){
	do{
		BBINT bbt_oldval=*bbt_target;
		if(bbAtomicCAS((int*)&(*bbt_target),(int)*bbt_target,(int)bbt_value)!=0){
			return bbt_oldval;
		}
	}while(!(0));
}

Dunno if the GCC optimizes it to the same final assembler code at the end.

[woollybah]

We could always force a minimum of C11 and implement support for C11's "atomic" primitives...

Rather than "Int", we'd need to define the variable as an atomic int, eg "AtomicInt", which you'd need to use instead for atomic operations. But then it would be guaranteed to work correctly.

[GWRon]

Most important is..if the MacOS stuff will rather sooner than later require a change. So before it surprises you..maybe be prepared. Regarding atomicint. I think an bbAtomicSet( variable:int var, value:int ) would already help. What it does in the background is maybe not that important It might even be interesting if it needs something for reading too (depending on memory layout the integer could be a 2 asm command thing..right?) Might with overloading be interesting for :long too. Before introducing c11 stuff (I understand that you are a bit eager to try it out :-)) it might be worth to check if the generated code (asm by gcc) uses multiple operations to in that CAS stuff. If this stuff is not that atomic as we thought... we need to also ensure that TCondVar etc are not relying on such a nonatomic thing.

[woollybah]

Well, there's nothing to directly replace it with on macOS, since the preferred replacement is to use atomic_fetch_add, which expects something like an atomic_int.

[GWRon]

Seems gcc offers atomic functions since 4.8
https://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/_005f_005fatomic-Builtins.html

So doesn't that mean we could use that (for non macos targets)?
The atomic store variant sounds also interesting (set without compare)

[GWRon]

Regarding "C11" ... You made some efforts to have it "C99" compatible (or so) to support old linux boxes with old GCC.
It might be potentially useful to have something working for them too.

Hmm ... but it might mean to keep it "potentially incorrect" as it is now.

Couldn't we introduce "Atomic" as a keyword?

Field Atomic i:int

Local Atomic i:int

Asking as introducing int_atomic might be simpler but I am not sure how often other stuff would be needed too (atomic types ).
Anyways. If you add some kind of int_atomic (or atomic_int to have it similar to the C naming scheme) do not forget about long, float and double and other >=32bit variants.