From patchwork Mon Jan 14 16:48:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 211840 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 B47812C009D for ; Tue, 15 Jan 2013 03:48:30 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1358786911; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=DK11G9ZAlfDHJ5Ui5gQ4y6m1e5E=; b=modYUkLOj5sduq2va28iZj8wxJkEV0fHrVtZV8/MSKGimm3Hqae2ushgsS47uv RAHrDqM3vnvtIbhvCOjz+zsG/FWalEq2y0a/7d5OEuZHjSx/kZIwxR9025xbHII7 z8A0AQT2X9k/K8Z+Gez6Gm0pVfbrPoQB1ZikhvbgFtkAQ= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=a42QmeNcRjbxP3GYuzjOD8MXvUmZxi/lELFEq6dYgReFh8L0L0quZYYwQyZGV1 oqhN67FfC49BsIR1eXEm2w2KwweaRQLvzoyjVuqKCz1G4NVduGjrcerkmCfNhKUN 0yTEu+OY7DDWiI22gFw082uGsPHE4lRB75jzt/lebyOas=; Received: (qmail 25017 invoked by alias); 14 Jan 2013 16:48:21 -0000 Received: (qmail 24984 invoked by uid 22791); 14 Jan 2013 16:48:17 -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, TW_OV, TW_ZJ X-Spam-Check-By: sourceware.org Received: from mail-oa0-f49.google.com (HELO mail-oa0-f49.google.com) (209.85.219.49) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Jan 2013 16:48:11 +0000 Received: by mail-oa0-f49.google.com with SMTP id l10so4105996oag.8 for ; Mon, 14 Jan 2013 08:48:10 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.141.70 with SMTP id rm6mr62692700obb.59.1358182090619; Mon, 14 Jan 2013 08:48:10 -0800 (PST) Received: by 10.182.153.201 with HTTP; Mon, 14 Jan 2013 08:48:10 -0800 (PST) In-Reply-To: <20130113222946.GX30577@one.firstfloor.org> References: <20130113203603.GS30577@one.firstfloor.org> <20130113221251.GW30577@one.firstfloor.org> <20130113222946.GX30577@one.firstfloor.org> Date: Mon, 14 Jan 2013 17:48:10 +0100 Message-ID: Subject: Re: [PATCH 2/2] Support __ATOMIC_HLE_RELEASE for __atomic_clear/store_n From: Uros Bizjak To: Andi Kleen Cc: gcc-patches@gcc.gnu.org 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 Sun, Jan 13, 2013 at 11:29 PM, Andi Kleen wrote: > On Sun, Jan 13, 2013 at 11:23:24PM +0100, Uros Bizjak wrote: >> On Sun, Jan 13, 2013 at 11:12 PM, Andi Kleen wrote: >> >> >> +(define_insn "atomic_store_1" >> >> >> + [(set (match_operand:ATOMIC 0 "memory_operand" "=m") >> >> >> + (unspec:ATOMIC [(match_operand:ATOMIC 1 "" "") >> >> >> + (match_operand:SI 2 "const_int_operand")] >> >> >> + UNSPEC_MOVA))] >> >> >> + "" >> >> >> + "%K2mov{}\t{%1, %0|%0, %1}") >> >> > >> >> > Is that the updated pattern you wanted? It looks similar to mine. >> >> >> >> Yes the attached patch actually implements all proposed fixes. >> > >> > Ok great. Can you just commit it then? It looks good to me. >> >> No problem, but what about this part: > > Right now it just means its silently ignored, no wrong code generated. > If people are ok with a new target hook I can add one. > There are some more bugs in this area, like PR55947 > > Giving a warning is imho less important than supporting this at all. > So I would prefer to not delay this patch. > >> >> diff --git a/gcc/builtins.c b/gcc/builtins.c >> index 2b615a1..c283869 100644 >> --- a/gcc/builtins.c >> +++ b/gcc/builtins.c >> @@ -5556,6 +5556,8 @@ expand_builtin_atomic_clear (tree exp) >> return const0_rtx; >> } >> >> + /* need target hook there to check for not hle acquire */ >> + >> if (HAVE_atomic_clear) >> { >> emit_insn (gen_atomic_clear (mem, model)); >> >> Middle-end support should be implemented before target support is >> committed. So, please figure out how to emit correct error on >> unsupported models and get middle-end patch reviewed first. We do get >> "Error: instruction `mov' after `xacquire' not allowed" assembler >> error with "xacquire movb $0,mem" asm, though. > > The sync.md code is only called for the acquire bit. > > The only case where it may happen I guess if someone sets both. This cannot happen, we reject code that sets both __HLE* flags. 2012-01-14 Uros Bizjak Andi Kleen PR target/55948 * config/i386/sync.md (atomic_store_1): New pattern. (atomic_store): Call atomic_store_1 for IX86_HLE_RELEASE memmodel flag. testsuite/ChangeLog 2012-01-14 Andi Kleen PR target/55948 * gcc.target/i386/hle-clear-rel.c: New file * gcc.target/i386/hle-store-rel.c: New file. I have committed attached patch to mainline SVN, after re-tested it on x86_64-pc-linux-gnu. Uros. Index: config/i386/sync.md =================================================================== --- config/i386/sync.md (revision 195152) +++ config/i386/sync.md (working copy) @@ -224,8 +224,12 @@ DONE; } - /* Otherwise use a normal store. */ - emit_move_insn (operands[0], operands[1]); + /* Otherwise use a store. */ + if (INTVAL (operands[2]) & IX86_HLE_RELEASE) + emit_insn (gen_atomic_store_1 (operands[0], operands[1], + operands[2])); + else + emit_move_insn (operands[0], operands[1]); } /* ... followed by an MFENCE, if required. */ if (model == MEMMODEL_SEQ_CST) @@ -233,6 +237,14 @@ DONE; }) +(define_insn "atomic_store_1" + [(set (match_operand:ATOMIC 0 "memory_operand" "=m") + (unspec:ATOMIC [(match_operand:ATOMIC 1 "" "") + (match_operand:SI 2 "const_int_operand")] + UNSPEC_MOVA))] + "" + "%K2mov{}\t{%1, %0|%0, %1}") + (define_insn_and_split "atomic_storedi_fpu" [(set (match_operand:DI 0 "memory_operand" "=m,m,m") (unspec:DI [(match_operand:DI 1 "register_operand" "x,m,?r")] Index: testsuite/gcc.target/i386/hle-clear-rel.c =================================================================== --- testsuite/gcc.target/i386/hle-clear-rel.c (revision 0) +++ testsuite/gcc.target/i386/hle-clear-rel.c (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mhle" } */ +/* { dg-final { scan-assembler "\[ \n\t\]+\(xrelease\|\.byte\[ \t\]+0xf3\)\[ \t\n\]+mov" } } */ + +void +hle_clear (char *p, int v) +{ + __atomic_clear (p, __ATOMIC_RELEASE | __ATOMIC_HLE_RELEASE); +} Index: testsuite/gcc.target/i386/hle-store-rel.c =================================================================== --- testsuite/gcc.target/i386/hle-store-rel.c (revision 0) +++ testsuite/gcc.target/i386/hle-store-rel.c (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mhle" } */ +/* { dg-final { scan-assembler "\[ \n\t\]+\(xrelease\|\.byte\[ \t\]+0xf3\)\[ \t\n\]+mov" } } */ + +void +hle_store (int *p, int v) +{ + __atomic_store_n (p, v, __ATOMIC_RELEASE | __ATOMIC_HLE_RELEASE); +}