From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29862 invoked by alias); 3 Sep 2013 17:31:36 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 29849 invoked by uid 89); 3 Sep 2013 17:31:36 -0000 Received: from hqemgate15.nvidia.com (HELO hqemgate15.nvidia.com) (216.228.121.64) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 03 Sep 2013 17:31:36 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: hqemgate15.nvidia.com Received: from hqnvupgp07.nvidia.com (Not Verified[216.228.121.13]) by hqemgate15.nvidia.com id ; Tue, 03 Sep 2013 10:31:24 -0700 Received: from hqemhub03.nvidia.com ([172.20.12.94]) by hqnvupgp07.nvidia.com (PGP Universal service); Tue, 03 Sep 2013 10:31:33 -0700 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Tue, 03 Sep 2013 10:31:33 -0700 Received: from HQMAIL02.nvidia.com ([172.20.150.111]) by hqemhub03.nvidia.com ([172.20.150.15]) with mapi; Tue, 3 Sep 2013 10:31:33 -0700 From: Abhishek Deb To: Joseph Myers , Dinar Temirbulatov CC: "libc-ports@sourceware.org" Date: Tue, 03 Sep 2013 17:31:00 -0000 Subject: RE: [Patch] ARM define atomic_exchange_acq/atomic_exchange_rel to __atomic_exchange_n Message-ID: <0E42B6C0C4628E48B8DF5D3F3C8FCA8898F2CF58CF@HQMAIL02.nvidia.com> References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-09/txt/msg00020.txt.bz2 =46rom the functionality point of view, the changes look ok. An objdump of the atomic_compare_and_exchange_bool_acq shows the following: 60: e1903f9f ldrex r3, [r0] 64: e1530002 cmp r3, r2 68: 1a000003 bne 7c 6c: e1804f91 strex r4, r1, [r0] 70: e3540000 cmp r4, #0 74: 1afffff9 bne 60 78: f57ff05f dmb sy As you can see it contains only one dmb in the end and not in the beginning= , which is in-line with acquire semantics to not allow any code hoisting. Abhishek -----Original Message----- From: Joseph Myers [mailto:joseph@codesourcery.com]=20 Sent: Monday, September 02, 2013 8:08 AM To: Dinar Temirbulatov Cc: libc-ports@sourceware.org; Abhishek Deb Subject: Re: [Patch] ARM define atomic_exchange_acq/atomic_exchange_rel to = __atomic_exchange_n On Mon, 2 Sep 2013, Dinar Temirbulatov wrote: > Hi, > Another version of change, I added > atomic_compare_and_exchange_val_acq/atomic_compare_and_exchange_val_re > l and=20 > atomic_compare_and_exchange_bool_acq/atomic_compare_and_exchange_boot_ > rel defenitions and also for gcc-4.7 and higher in the case of=20 > unsupported atomic compare and swap operation, it uses the kernel=20 > helper inlines. > Tested on arm a9 with no new regressions. Ok to commit? > Oh, sorry. I missed to attach the change. Here it is. For subsequent patch revisions, please note there should be an extra space = after "#" for preprocessor directives inside #if conditionals, one per leve= l of #if nesting (other than toplevel multiple-inclusion guards) - which me= ans that if conditioning existing code, you need to adjust directives insid= e that code. This patch appears to have too much duplication. For example, you duplicat= e the definition of __arm_assisted_compare_and_exchange_val_32_acq - but that should not need any extra conditionals at all (beyond the existi= ng #ifndef __arm_assisted_compare_and_exchange_val_32_acq), there's no reas= on ever not to define it. Similarly, you duplicate __arch_compare_and_exch= ange_val_64_acq, but with proper #if structure there should only need to be= one copy of the version that uses __arm_link_error. What I think you should aim for is that each definition, or small group of = definitions, uses conditionals in the form #if __GNUC_PREREQ (4, 7) && defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 /* Version using __atomic_*. */ #elif defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 /* Version using __sync_*. */ #else /* Version using __arm_assisted_*. */ #endif with cases omitted if not useful for that particular macro (this may includ= e some macros not being defined at all in some cases). So don't insert any= global conditionals affecting all the existing definitions at all - look a= t each block of conditionals and add a third case as needed, along with any= new macros (again with conditionals in that form) that are appropriate. Where you use abort () in some definitions, use __arm_link_error () instead= , like for the existing definitions. -- Joseph S. Myers joseph@codesourcery.com