From patchwork Tue Nov 29 12:27:02 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Modra X-Patchwork-Id: 128280 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id AACB41007D1 for ; Tue, 29 Nov 2011 23:27:36 +1100 (EST) Received: (qmail 27870 invoked by alias); 29 Nov 2011 12:27:31 -0000 Received: (qmail 27858 invoked by uid 22791); 29 Nov 2011 12:27:26 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 29 Nov 2011 12:27:11 +0000 Received: by yenl4 with SMTP id l4so2035223yen.20 for ; Tue, 29 Nov 2011 04:27:10 -0800 (PST) Received: by 10.236.161.65 with SMTP id v41mr70363153yhk.42.1322569629396; Tue, 29 Nov 2011 04:27:09 -0800 (PST) Received: from bubble.grove.modra.org ([115.187.252.19]) by mx.google.com with ESMTPS id c44sm56496132yhm.5.2011.11.29.04.27.06 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 29 Nov 2011 04:27:08 -0800 (PST) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 47026170C2BF; Tue, 29 Nov 2011 22:57:02 +1030 (CST) Date: Tue, 29 Nov 2011 22:57:02 +1030 From: Alan Modra To: Richard Henderson Cc: Jakub Jelinek , gcc-patches@gcc.gnu.org Subject: Re: Use atomics in libgomp mutex Message-ID: <20111129122702.GQ7827@bubble.grove.modra.org> Mail-Followup-To: Richard Henderson , Jakub Jelinek , gcc-patches@gcc.gnu.org References: <20111125000328.GD5085@bubble.grove.modra.org> <20111125073839.GN27242@tyan-ft48-01.lab.bos.redhat.com> <20111127225720.GB7827@bubble.grove.modra.org> <4ED4365D.2060209@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4ED4365D.2060209@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Mon, Nov 28, 2011 at 05:33:17PM -0800, Richard Henderson wrote: > On 11/27/2011 02:57 PM, Alan Modra wrote: > > This is the mutex part. Depends on > > http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02454.html for MEMMODEL_* > > values. > > > > * config/linux/mutex.h: Use atomic rather than sync builtins. > > * config/linux/mutex.c: Likewise. Comment. Use -1 for waiting state. > > * config/linux/omp-lock.h: Comment fix. > > * config/linux/arm/mutex.h: Delete. > > * config/linux/powerpc/mutex.h: Delete. > > * config/linux/ia64/mutex.h: Delete. > > * config/linux/mips/mutex.h: Delete. > > Looks good modulo _4/_n and using the success return of __atomic_compare_exchange. Did you see my comment about the OpenMP spec requirement that critical, ordered and parallel regions have a flush on entry and on exit? So should I commit a version with static inline void gomp_mutex_lock (gomp_mutex_t *mutex) { int oldval = 0; /* FIXME: This should just be MEMMODEL_ACQUIRE, MEMMODEL_RELAXED but GOMP_critical_start and other functions rely on the lock acquisition doing a flush. See OpenMP API version 3.1 section 2.8.6 flush Construct. */ if (__builtin_expect (!__atomic_compare_exchange_n (mutex, &oldval, 1, false, MEMMODEL_ACQ_REL, MEMMODEL_ACQUIRE), 0)) gomp_mutex_lock_slow (mutex, oldval); } or as posted in http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02455.html? Perhaps with the following as well * ordered.c (gomp_ordered_sync): Add MEMMODEL_RELEASE fence. * critical.c (GOMP_critical_start): Likewise. I think we're OK on parallel regions by virtue of gomp_barrier_wait, but I may not know what I'm talking about. This stuff fries my brains. Index: ordered.c =================================================================== --- ordered.c (revision 181770) +++ ordered.c (working copy) @@ -199,6 +199,9 @@ gomp_ordered_sync (void) if (team == NULL || team->nthreads == 1) return; + /* There is an implicit flush on entry to an ordered region. */ + __atomic_thread_fence (MEMMODEL_RELEASE); + /* ??? I believe it to be safe to access this data without taking the ws->lock. The only presumed race condition is with the previous thread on the queue incrementing ordered_cur such that it points Index: critical.c =================================================================== --- critical.c (revision 181770) +++ critical.c (working copy) @@ -33,6 +33,8 @@ static gomp_mutex_t default_lock; void GOMP_critical_start (void) { + /* There is an implicit flush on entry to a critical region. */ + __atomic_thread_fence (MEMMODEL_RELEASE); gomp_mutex_lock (&default_lock); }