[Vm-dev] Clarification needed for sqCompareAndSwapRes
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Mon Apr 21 22:40:07 UTC 2014
In platforms/Cross/vm/sqAtomicOps.h a comment tells
* sqCompareAndSwapRes(var,old,new,res) arranges atomically that if var's
value
* is equal to old, then var's value is set to new, and that in any case, res
* is set to the previous value of var.
But for some implementation, if var==old, res is first set to var (old)
then to new which is the new value of var:
#if defined(__GNUC__) && (defined(i386) || defined(__i386) ||
defined(__i386__) || defined(_X86_))
#ifdef TARGET_OS_IS_IPHONE
# define sqCompareAndSwap(var,old,new) OSAtomicCompareAndSwap32(old, new, &var)
/* N.B. This is not atomic in fetching var's old value :( */
# define sqCompareAndSwapRes(var,old,new,res) do { res = var; if
(OSAtomicCompareAndSwap32(old, new, &var)) res = new; } while (0)
Indeed, comparedAndSwap32 will answer true if var==old as explained here
https://www.mikeash.com/pyblog/friday-qa-2011-03-04-a-tour-of-osatomic.html
That is going to be a problem in lockSignalQueue in
./platforms/Cross/vm/sqExternalSemaphores.c
because variable old will then be 1 in all cases and the loop won't exit
once the lock acquired:
static inline void lockSignalQueue()
{
volatile int old;
/* spin to obtain a lock */
do {
sqLowLevelMFence();
sqCompareAndSwapRes(sigLock, 0, 1, old );
} while (old != 0);
}
Knowing we are on iphone, I would just write it:
static inline void lockSignalQueue()
{
/* spin to obtain a lock */
do {
sqLowLevelMFence();
} while (! OSAtomicCompareInt32(sigLock, 0, 1));
}
Esteban "fixed" that by swapping the arguments in the macrosqCompareAndSwapRes
But IMO, this is wrong, as I understand it, it completely desactivates the
lock...
I did not know about the problem, but found the diff with Eliot's branch
very suspicious so I opened
https://pharo.fogbugz.com/f/cases/13199/Stinky-argument-inversion-of-atomic-swap-MACRO-for-iphone-gcc-branch
and pulled a request https://github.com/pharo-project/pharo-vm/pull/38 for
restoring (the supposedly wrong) Eliot's version.
Now I need some guru advice, because this kind of code is brainfucking
(especially when old in sender is not old in sendee).
Do you agree that TARGET_OS_IS_IPHONE branch is wrong?
Do you agree that Esteban's fix is wrong?
What do you suggest for a fix?
Thanks
Nicolas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140422/65297a83/attachment.htm
More information about the Vm-dev
mailing list