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

[David Chisnall]

On 31 Aug 2013, at 12:30, Ed Schouten <ed at 80386.nl> wrote:

> 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.

I completely agree with all of this.  There are lots of things in the layering in LLVM that I don't like that place too much target-specific knowledge in the front ends.  

> 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

libcxxrt, in particular, has the constraint that it must also compile with gcc and Path64, so patches of this nature need careful testing.  Although this would fix the issue in the tree, a number of ports explicitly call the __sync_* builtins, including some that have USE_GCC set, so even fixing clang would not address this.

David