From patchwork Mon Jan 14 19:21:16 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 211865 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 7DF452C00A5 for ; Tue, 15 Jan 2013 06:21:29 +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=1358796090; 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=hZNXHt35ca58If4BytMiWKbXr2Y=; b=nb2s1DuWFSebSXhhLhGqiJugC9XbXHLbODIcRsKRA5s8fKmFYD6/BK8W9LX6Xr 27Q9N5LXPVXy+KjA0WC+Z0lOHx9IaKOoQ4CCPO1uQ3Dnq7/DBg0l4G/yW/6pc5Ls urp3lAiWDlA9zG/KAHOBlrZz/ch5dZgegnx1r6slHgeHE= 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=PL4HXRzJJTTRAQps9jGoF/hfjM8lZ1OC1JiOJI4hGuOcN0Hadn4nratnC7iWVz GsbMozXw0z30Nbsi72EB+6dwhu1rnrjNrYmb03LtodjYuzyMOWtwqBw2AGIUkxYh sLB/g/NtX2lwKa3hvhOcuUb2t6lyI5bjM8YEGbyvpTA2k=; Received: (qmail 12604 invoked by alias); 14 Jan 2013 19:21:24 -0000 Received: (qmail 12593 invoked by uid 22791); 14 Jan 2013 19:21:23 -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_PX 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 19:21:17 +0000 Received: by mail-oa0-f49.google.com with SMTP id l10so4289306oag.8 for ; Mon, 14 Jan 2013 11:21:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.60.32.235 with SMTP id m11mr54581226oei.129.1358191276704; Mon, 14 Jan 2013 11:21:16 -0800 (PST) Received: by 10.182.153.201 with HTTP; Mon, 14 Jan 2013 11:21:16 -0800 (PST) In-Reply-To: <20130114190153.GD30577@one.firstfloor.org> References: <20130113203603.GS30577@one.firstfloor.org> <20130113221251.GW30577@one.firstfloor.org> <20130113222946.GX30577@one.firstfloor.org> <20130114180601.GA30577@one.firstfloor.org> <20130114190153.GD30577@one.firstfloor.org> Date: Mon, 14 Jan 2013 20:21:16 +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 Mon, Jan 14, 2013 at 8:01 PM, Andi Kleen wrote: >> >> This cannot happen, we reject code that sets both __HLE* flags. >> > >> > BTW I found more HLE bugs, it looks like some of the fetch_op_* >> > patterns do not match always and fall back to cmpxchg, which >> > does not generate HLE code correctly. Not fully sure what's >> > wrong, can you spot any obvious problems? You changed the >> > >> > (define_insn "atomic_" >> > >> > pattern last. >> >> I don't think this is a target problem, these insns work as expected >> and are covered by extensive testsuite in gcc.target/i386/hle-*.c. > > Well the C++ test cases I wrote didn't work. It may be related to > how complex the program is. Simple calls as in the original > test suite seem to work. > > e.g. instead of xacquire lock and ... it ended up with a cmpxchg loop > (which I think is a fallback path). The cmpxchg loop didn't include > a HLE prefix (and simply adding one is not enoigh, would need more > changes for successfull elision) > > Before HLE the cmpxchg code was correct, just somewhat inefficient. > Even with HLE it is technically correct, just it'll never elide. I'd start with attached (mechanical) patch that just blindly adds masks where memory model is checked. Please note that ATOMIC_HLE modifies high bits of the model, so these checks fail in presence of HLE modifiers. Uros. Index: emit-rtl.c =================================================================== --- emit-rtl.c (revision 195152) +++ emit-rtl.c (working copy) @@ -6014,7 +6014,7 @@ insn_file (const_rtx insn) bool need_atomic_barrier_p (enum memmodel model, bool pre) { - switch (model) + switch (model & MEMMODEL_MASK) { case MEMMODEL_RELAXED: case MEMMODEL_CONSUME: Index: optabs.c =================================================================== --- optabs.c (revision 195152) +++ optabs.c (working copy) @@ -7008,9 +7008,9 @@ maybe_emit_sync_lock_test_and_set (rtx target, rtx exists, and the memory model is stronger than acquire, add a release barrier before the instruction. */ - if (model == MEMMODEL_SEQ_CST - || model == MEMMODEL_RELEASE - || model == MEMMODEL_ACQ_REL) + if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST + || (model & MEMMODEL_MASK) == MEMMODEL_RELEASE + || (model & MEMMODEL_MASK) == MEMMODEL_ACQ_REL) expand_mem_thread_fence (model); if (icode != CODE_FOR_nothing) @@ -7388,7 +7388,7 @@ expand_mem_thread_fence (enum memmodel model) { if (HAVE_mem_thread_fence) emit_insn (gen_mem_thread_fence (GEN_INT (model))); - else if (model != MEMMODEL_RELAXED) + else if ((model & MEMMODEL_MASK) != MEMMODEL_RELAXED) { if (HAVE_memory_barrier) emit_insn (gen_memory_barrier ()); @@ -7412,7 +7412,7 @@ expand_mem_signal_fence (enum memmodel model) { if (HAVE_mem_signal_fence) emit_insn (gen_mem_signal_fence (GEN_INT (model))); - else if (model != MEMMODEL_RELAXED) + else if ((model & MEMMODEL_MASK) != MEMMODEL_RELAXED) { /* By default targets are coherent between a thread and the signal handler running on the same thread. Thus this really becomes a @@ -7467,7 +7467,7 @@ expand_atomic_load (rtx target, rtx mem, enum memm target = gen_reg_rtx (mode); /* For SEQ_CST, emit a barrier before the load. */ - if (model == MEMMODEL_SEQ_CST) + if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST) expand_mem_thread_fence (model); emit_move_insn (target, mem); @@ -7513,7 +7513,7 @@ expand_atomic_store (rtx mem, rtx val, enum memmod if (maybe_expand_insn (icode, 2, ops)) { /* lock_release is only a release barrier. */ - if (model == MEMMODEL_SEQ_CST) + if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST) expand_mem_thread_fence (model); return const0_rtx; } @@ -7540,7 +7540,7 @@ expand_atomic_store (rtx mem, rtx val, enum memmod emit_move_insn (mem, val); /* For SEQ_CST, also emit a barrier after the store. */ - if (model == MEMMODEL_SEQ_CST) + if ((model & MEMMODEL_MASK) == MEMMODEL_SEQ_CST) expand_mem_thread_fence (model); return const0_rtx;