From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27163 invoked by alias); 14 Jan 2013 16:53:21 -0000 Received: (qmail 27147 invoked by uid 22791); 14 Jan 2013 16:53:20 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-oa0-f41.google.com (HELO mail-oa0-f41.google.com) (209.85.219.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Jan 2013 16:53:11 +0000 Received: by mail-oa0-f41.google.com with SMTP id k14so4248161oag.0 for ; Mon, 14 Jan 2013 08:53:10 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.54.102 with SMTP id i6mr61309698obp.67.1358182390383; Mon, 14 Jan 2013 08:53:10 -0800 (PST) Received: by 10.182.153.201 with HTTP; Mon, 14 Jan 2013 08:53:10 -0800 (PST) In-Reply-To: <1358177908-31624-1-git-send-email-andi@firstfloor.org> References: <1358177908-31624-1-git-send-email-andi@firstfloor.org> Date: Mon, 14 Jan 2013 16:53:00 -0000 Message-ID: Subject: Re: [PATCH] Let target get_memmodel know about stores/barriers From: Uros Bizjak To: Andi Kleen Cc: gcc-patches@gcc.gnu.org, Andi Kleen Content-Type: text/plain; charset=ISO-8859-1 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 X-SW-Source: 2013-01/txt/msg00708.txt.bz2 On Mon, Jan 14, 2013 at 4:38 PM, Andi Kleen wrote: > For HLE stores are only valid with __ATOMIC_HLE_RELEASE. The middle end > didn't know this. This adds a new parameter to the get_memmodel target > hook to distingush stores and give an warning for acquire stores. > > I also did a similar check for the barriers where HLE is not useful > at all. > > Fixes one todo in the earlier hle release patch. > > gcc/: > 2013-01-13 Andi Kleen > > PR target/55948 > * builtins.c (get_memmodel): Pass store, barrier parameters to target > hook. > (expand_builtin_atomic_exchange, > expand_builtin_atomic_compare_exchange, > expand_builtin_atomic_load, > expand_builtin_atomic_store, > expand_builtin_atomic_fetch_op, > expand_builtin_atomic_test_and_set, > expand_builtin_atomic_test_lock, > expand_builtin_atomic_clear, > expand_builtin_atomic_thread_fence): Pass store, barrier > parameters to get_memmodel. > * config/i386/i386.c (ix86_memmodel_check): Add store, barrier warning. > * doc/tm.texi (TARGET_GET_MEMMODEL): Add store, barrier parameters. > * doc/tm.exit.in (TARGET_GET_MEMMODEL): Dito. > * target.def (TARGET_GET_MEMMODEL): Dito. > --- > gcc/builtins.c | 30 +++++++++++++++--------------- > gcc/config/i386/i386.c | 21 +++++++++++++++++++-- > gcc/doc/tm.texi | 6 ++++-- > gcc/doc/tm.texi.in | 4 +++- > gcc/target.def | 3 ++- > 5 files changed, 43 insertions(+), 21 deletions(-) > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 4f778c1..50b6297 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -42076,14 +42076,31 @@ ix86_destroy_cost_data (void *data) > free (data); > } > > -/* Validate target specific memory model bits in VAL. */ > +/* Validate target specific memory model bits in VAL. When > + STORE is true this for a store. When BARRIER is true > + this is for a barrier. */ > > static unsigned HOST_WIDE_INT > -ix86_memmodel_check (unsigned HOST_WIDE_INT val) > +ix86_memmodel_check (unsigned HOST_WIDE_INT val, bool store, > + bool barrier) > { > unsigned HOST_WIDE_INT model = val & MEMMODEL_MASK; > bool strong; > > + if (barrier && (val & (IX86_HLE_ACQUIRE|IX86_HLE_RELEASE))) > + { > + warning (OPT_Winvalid_memory_model, > + "__ATOMIC_HLE_ACQUIRE or __ATOMIC_HLE_RELEASE not valid for barriers."); No dot at the end of warning sentence. > + return MEMMODEL_SEQ_CST; > + } > + > + if (store && (val & IX86_HLE_ACQUIRE)) > + { > + warning (OPT_Winvalid_memory_model, > + "__ATOMIC_HLE_ACQUIRE not valid for stores."); Also here. The target part is OK, the rest needs a review from a middle-end reviewer. Thanks, Uros.