[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