From owner-svn-src-all@FreeBSD.ORG Sat Aug 31 11:30:04 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 1979BAAD; Sat, 31 Aug 2013 11:30:04 +0000 (UTC) (envelope-from edschouten@gmail.com) Received: from mail-vc0-x22e.google.com (mail-vc0-x22e.google.com [IPv6:2607:f8b0:400c:c03::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 97EFE2ADF; Sat, 31 Aug 2013 11:30:03 +0000 (UTC) Received: by mail-vc0-f174.google.com with SMTP id gd11so1962741vcb.33 for ; Sat, 31 Aug 2013 04:30:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=lTGv9O/ctrb+yXn4DSACV4/of1dHe0vBZybfk/zA4Lk=; b=vrjWtxmZnWQGtmUxXUf5pdXrxkH8jM55K4SFFk1Qj6fBHksg45cLTLjY2nePn899AL rERGCYIDIRAeYYXVKF/zUiiFKHXp8cqtaBZZ8D6o/ttXMAkgShavBdCTH1rTaAgZgGx7 TE+EUxCJoJn41vEQnAmxAtCeVUTgwZt3FpSctyV7DwDwRcraOH5L97BdTXWD7koUCuRH fLen/o9kwlw+40IKqpYr50ZuU6mBPSj9KsZeFuL7JQ+ptfwSGLuMSkrY7SZyhmNSw2U6 0ATqS9YeuajFBP5MkR4dIjgAermqujR6UYzj+pscSSmGdAFmukxrOVYHeOVA+dseTO2d SLZA== MIME-Version: 1.0 X-Received: by 10.58.137.167 with SMTP id qj7mr12817112veb.1.1377948602413; Sat, 31 Aug 2013 04:30:02 -0700 (PDT) Sender: edschouten@gmail.com Received: by 10.220.115.206 with HTTP; Sat, 31 Aug 2013 04:30:02 -0700 (PDT) In-Reply-To: <201308310850.r7V8ojQX022383@svn.freebsd.org> References: <201308310850.r7V8ojQX022383@svn.freebsd.org> Date: Sat, 31 Aug 2013 13:30:02 +0200 X-Google-Sender-Auth: 6g2eWwSy80uHPGHSiZ9RXMAxQdQ Message-ID: Subject: Re: svn commit: r255092 - in head: lib/libcompiler_rt sys/arm/arm From: Ed Schouten To: David Chisnall Content-Type: text/plain; charset=UTF-8 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 31 Aug 2013 11:30:04 -0000 2013/8/31 David Chisnall : > 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 or . 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