Re: [PATCH v2] Use CC atomic builtins as a fallback

Martin Pitt <mpitt(at)debian(dot)org> writes:
> Tom Lane [2011-12-20 21:14 -0500]:
>> Another thing that is bothering me is that according to the gcc
>> manual, eg here,
>> http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html
>> __sync_lock_test_and_set is nominally provided for datatypes 1, 2,
>> 4, or 8 bytes in length, but the underlying hardware doesn't
>> necessarily support all those widths natively. If you pick the
>> wrong width then you don't get an inline operation at all, but a
>> call to some possibly inefficient library subroutine.

> It's even worse. Our original version of the patch used a char, and
> while that worked fine on the older Babbage board, it completely broke
> at runtime on e. g. a Panda board. It wasn't just slow, it caused
> hangs and test failures all over the place. With int it works
> flawlessly.

Yeah, that was another thing I found worrisome while googling: there
were a disturbingly large number of claims that __sync_lock_test_and_set
and/or __sync_lock_release were flat-out broken on various combinations
of gcc version and platform. After reading that, there is no way at all
that I'd accept your original patch to use these functions everywhere.

For the moment I'm inclined to consider using these functions *only* on
ARM, so as to limit our exposure to such bugs. That would also limit
the risk of using an inappropriate choice of lock width.

regards, tom lane