svn commit: r255092 - in head: lib/libcompiler_rt sys/arm/arm

[Ed Schouten]

2013/8/31 David Chisnall <theraven at freebsd.org>:
>   Reviewed by:  ed

Just for the record: though I did review this patch, but did not agree
with the approach taken. Though I have to confess that due all the
case distinctions this code isn't the cleanest out there, the
#pragmas/__strong_references make it even less readable.

In my opinion, this should have been fixed as follows:

1. Fix LLVM/Clang.

Never ever let LLVM/Clang emit calls to __sync_*. You can easily
implement the __sync_* interface on top of __atomic_*. What baffles
me, is that the calls to __sync_* are emitted by LLVM
(SelectionDAGLegalize::ExpandAtomic), while Clang is responsible for
emitting the __atomic_* calls (CodeGenFunction::EmitAtomicExpr).

All of this should have been pushed down into LLVM in the first place.
It is currently hard to implement your own programming language on top
of LLVM that is capable of doing atomic operations the same way
C11/C++11 does them. You either have to use the __sync_* intrinsics or
duplicate all the code that emits the libcalls.

I've noticed that due to this separation of doing __sync_* in LLVM and
__atomic_* in Clang, the behaviour of Clang can sometimes be
incredibly counter-intuitive. For example, both LLVM and Clang need a
similar piece of code to determine whether hardware atomics are
available. I've noticed that if this logic is not identical, Clang
does some really weird things. If you call __atomic_* functions and
Clang thinks we have hardware atomics, but LLVM thinks we do not, we
may end up emitting calls to __sync_* instead.

Furthermore, the duplication of code between EmitAtomicExpr,
EmitAtomicStore and EmitAtomicLoad leads to the problem that
EmitAtomicExpr properly emits power-of-two-sized calls for most
primitive datatypes, while EmitAtomicStore and EmitAtomicLoad do not.

2. Fix the consumers.

Right now there are only two pieces of code in our tree (libc++ and
libcxxrt) that emit calls to __sync_* unconditionally. All the other
code either uses <machine/atomic.h> or <stdatomic.h>. These pieces of
code could have been ported to use C11 atomics with a similar amount
of effort.

For libcxxrt you could even argue that this should have done in the
first place, because it already used __atomic_* calls in certain
source files.

Example patch for libcxxrt:

http://80386.nl/pub/libcxxrt.txt

-- 
Ed Schouten <ed at 80386.nl>