From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2617 invoked by alias); 19 Nov 2018 07:38:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2593 invoked by uid 89); 19 Nov 2018 07:38:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=2018-11-12, 20181112, converts, H*c:alternative X-HELO: mail-yw1-f67.google.com Received: from mail-yw1-f67.google.com (HELO mail-yw1-f67.google.com) (209.85.161.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Nov 2018 07:37:54 +0000 Received: by mail-yw1-f67.google.com with SMTP id i20so5175169ywc.5; Sun, 18 Nov 2018 23:37:54 -0800 (PST) MIME-Version: 1.0 References: <20181109205551.GN11625@tucnak> <20181112140508.15215-1-blomqvist.janne@gmail.com> In-Reply-To: <20181112140508.15215-1-blomqvist.janne@gmail.com> From: Janne Blomqvist Date: Mon, 19 Nov 2018 07:38:00 -0000 Message-ID: Subject: Re: [PATCH] Replace sync builtins with atomic builtins To: Fortran List , GCC Patches Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-11/txt/msg01601.txt.bz2 PING! On Mon, Nov 12, 2018 at 4:05 PM Janne Blomqvist wrote: > The old __sync builtins have been deprecated for a long time now in > favor of the __atomic builtins following the C++11/C11 memory model. > This patch converts libgfortran to use the modern __atomic builtins. > > At the same time I weakened the consistency to relaxed for > incrementing and decrementing the counter, and acquire-release when > decrementing to check whether the counter is 0 and the unit can be > freed. This is similar to e.g. std::shared_ptr in C++. Jakub, as the > original author of the algorithm, do you concur? > > Regtested on x86_64-pc-linux-gnu, Ok for trunk? > > libgfortran/ChangeLog: > > 2018-11-12 Janne Blomqvist > > * acinclude.m4 (LIBGFOR_CHECK_ATOMIC_FETCH_ADD): Rename and test > presence of atomic builtins instead of sync builtins. > * configure.ac (LIBGFOR_CHECK_ATOMIC_FETCH_ADD): Call new test. > * io/io.h (inc_waiting_locked): Use __atomic_fetch_add. > (predec_waiting_locked): Use __atomic_add_fetch. > (dec_waiting_unlocked): Use __atomic_fetch_add. > * config.h.in: Regenerated. > * configure: Regenerated. > --- > libgfortran/acinclude.m4 | 20 ++++++++++---------- > libgfortran/configure.ac | 4 ++-- > libgfortran/io/io.h | 24 ++++++++++++++++++------ > 3 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/libgfortran/acinclude.m4 b/libgfortran/acinclude.m4 > index dd5429ac0d2..5b0c094e716 100644 > --- a/libgfortran/acinclude.m4 > +++ b/libgfortran/acinclude.m4 > @@ -59,17 +59,17 @@ extern void bar(void) __attribute__((alias("foo")));]], > [Define to 1 if the target supports __attribute__((alias(...))).]) > fi]) > > -dnl Check whether the target supports __sync_fetch_and_add. > -AC_DEFUN([LIBGFOR_CHECK_SYNC_FETCH_AND_ADD], [ > - AC_CACHE_CHECK([whether the target supports __sync_fetch_and_add], > - libgfor_cv_have_sync_fetch_and_add, [ > +dnl Check whether the target supports __atomic_fetch_add. > +AC_DEFUN([LIBGFOR_CHECK_ATOMIC_FETCH_ADD], [ > + AC_CACHE_CHECK([whether the target supports __atomic_fetch_add], > + libgfor_cv_have_atomic_fetch_add, [ > AC_LINK_IFELSE([AC_LANG_PROGRAM([[int foovar = 0;]], [[ > -if (foovar <= 0) return __sync_fetch_and_add (&foovar, 1); > -if (foovar > 10) return __sync_add_and_fetch (&foovar, -1);]])], > - libgfor_cv_have_sync_fetch_and_add=yes, > libgfor_cv_have_sync_fetch_and_add=no)]) > - if test $libgfor_cv_have_sync_fetch_and_add = yes; then > - AC_DEFINE(HAVE_SYNC_FETCH_AND_ADD, 1, > - [Define to 1 if the target supports __sync_fetch_and_add]) > +if (foovar <= 0) return __atomic_fetch_add (&foovar, 1, __ATOMIC_ACQ_REL); > +if (foovar > 10) return __atomic_add_fetch (&foovar, -1, > __ATOMIC_ACQ_REL);]])], > + libgfor_cv_have_atomic_fetch_add=yes, > libgfor_cv_have_atomic_fetch_add=no)]) > + if test $libgfor_cv_have_atomic_fetch_add = yes; then > + AC_DEFINE(HAVE_ATOMIC_FETCH_ADD, 1, > + [Define to 1 if the target supports __atomic_fetch_add]) > fi]) > > dnl Check for pragma weak. > diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac > index 76007d38f6f..30ff8734760 100644 > --- a/libgfortran/configure.ac > +++ b/libgfortran/configure.ac > @@ -608,8 +608,8 @@ fi > LIBGFOR_CHECK_ATTRIBUTE_VISIBILITY > LIBGFOR_CHECK_ATTRIBUTE_ALIAS > > -# Check out sync builtins support. > -LIBGFOR_CHECK_SYNC_FETCH_AND_ADD > +# Check out atomic builtins support. > +LIBGFOR_CHECK_ATOMIC_FETCH_ADD > > # Check out #pragma weak. > LIBGFOR_GTHREAD_WEAK > diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h > index 902eb412848..282c1455763 100644 > --- a/libgfortran/io/io.h > +++ b/libgfortran/io/io.h > @@ -961,8 +961,8 @@ internal_proto(free_ionml); > static inline void > inc_waiting_locked (gfc_unit *u) > { > -#ifdef HAVE_SYNC_FETCH_AND_ADD > - (void) __sync_fetch_and_add (&u->waiting, 1); > +#ifdef HAVE_ATOMIC_FETCH_ADD > + (void) __atomic_fetch_add (&u->waiting, 1, __ATOMIC_RELAXED); > #else > u->waiting++; > #endif > @@ -971,8 +971,20 @@ inc_waiting_locked (gfc_unit *u) > static inline int > predec_waiting_locked (gfc_unit *u) > { > -#ifdef HAVE_SYNC_FETCH_AND_ADD > - return __sync_add_and_fetch (&u->waiting, -1); > +#ifdef HAVE_ATOMIC_FETCH_ADD > + /* Note that the pattern > + > + if (predec_waiting_locked (u) == 0) > + // destroy u > + > + could be further optimized by making this be an __ATOMIC_RELEASE, > + and then inserting a > + > + __atomic_thread_fence (__ATOMIC_ACQUIRE); > + > + inside the branch before destroying. But for now, lets keep it > + simple. */ > + return __atomic_add_fetch (&u->waiting, -1, __ATOMIC_ACQ_REL); > #else > return --u->waiting; > #endif > @@ -981,8 +993,8 @@ predec_waiting_locked (gfc_unit *u) > static inline void > dec_waiting_unlocked (gfc_unit *u) > { > -#ifdef HAVE_SYNC_FETCH_AND_ADD > - (void) __sync_fetch_and_add (&u->waiting, -1); > +#ifdef HAVE_ATOMIC_FETCH_ADD > + (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); > #else > __gthread_mutex_lock (&unit_lock); > u->waiting--; > -- > 2.17.1 > > -- Janne Blomqvist