From patchwork Wed Dec 21 12:49:37 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oleg Endo X-Patchwork-Id: 132625 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 1C2EDB7110 for ; Wed, 21 Dec 2011 23:50:10 +1100 (EST) Received: (qmail 9736 invoked by alias); 21 Dec 2011 12:50:05 -0000 Received: (qmail 9721 invoked by uid 22791); 21 Dec 2011 12:50:03 -0000 X-SWARE-Spam-Status: No, hits=-3.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, UNPARSEABLE_RELAY X-Spam-Check-By: sourceware.org Received: from mailout04.t-online.de (HELO mailout04.t-online.de) (194.25.134.18) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Dec 2011 12:49:45 +0000 Received: from fwd09.aul.t-online.de (fwd09.aul.t-online.de ) by mailout04.t-online.de with smtp id 1RdLc7-0005s9-OI; Wed, 21 Dec 2011 13:49:43 +0100 Received: from [192.168.0.104] (Ee0FfuZeohySSguKu+Pow72wByRxL-trnCI72fo5MfjwaycHK28bxEh-WBL5AWLQjP@[93.218.168.85]) by fwd09.t-online.de with esmtp id 1RdLc2-27pqls0; Wed, 21 Dec 2011 13:49:38 +0100 Subject: Re: [PATCH] atomic test and set re-org. From: Oleg Endo To: Andrew MacLeod Cc: gcc-patches , Richard Henderson In-Reply-To: <4ECEAE1B.6070304@redhat.com> References: <4ECEAE1B.6070304@redhat.com> Date: Wed, 21 Dec 2011 13:49:37 +0100 Message-ID: <1324471777.18753.528.camel@yam-132-YW-E178-FTW> Mime-Version: 1.0 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 Thu, 2011-11-24 at 15:50 -0500, Andrew MacLeod wrote: > This patch adds missing pattern support for atomic_test_and_set and > atomic_clear operations. It also restructures the code for > atomic_test_and_set, atomic_exchange, and __sync_lock_test_and_set so > that it is easier to read and tries things in a rational manner. > > bootstrapped on x86_64-unknown-linux-gnu with no new regressions. > > Andrew I've tried adding support for the new atomic_test_and_set to the SH target and ran into problems when compiling code that would use the return value of the atomic_test_and_set pattern. The attached patch fixes this problem and also wraps the calls to gen_atomic_test_and_set in the way it is done by other maybe_emit_* functions. Could you please have a look at it? BTW, currently the only target utilizing the new atomic_test_and_set is SPARC. However, as far as I've observed an expander like (define_expand "atomic_test_and_set" will never be used, because currently the atomic_test_and_set never has a mode in its name. As far as I understand it, atomic_test_and_set is supposed to operate on a bool only and thus no additional mode info is needed. Or is something else missing? Cheers, Oleg 2011-12-21 Oleg Endo * optab.c (maybe_emit_atomic_test_and_set): New function. (expand_sync_lock_test_and_set): Use it. (expand_atomic_test_and_set): Use it. Index: gcc/optabs.c =================================================================== --- gcc/optabs.c (revision 182531) +++ gcc/optabs.c (working copy) @@ -7436,10 +7436,35 @@ return NULL_RTX; } +/* This function tries to emit an atomic test and set operation using + __atomic_test_and_set, if it is defined in the target. */ + +static rtx +maybe_emit_atomic_test_and_set (rtx target, rtx mem, enum memmodel model) +{ #ifndef HAVE_atomic_test_and_set -#define HAVE_atomic_test_and_set 0 -#define gen_atomic_test_and_set(x,y,z) (gcc_unreachable (), NULL_RTX) + + return NULL_RTX; + +#else + + /* Allow the target to use a different mode than mem for storing + the previous value in some target specific way, as in + compare_and_swap. + This allows test and set conditional operations to be combined + better with surrounding code. */ + enum insn_code icode = CODE_FOR_atomic_test_and_set; + enum machine_mode imode = insn_data[icode].operand[0].mode; + + if (target == NULL_RTX || GET_MODE (target) != imode) + target = gen_reg_rtx (imode); + + emit_insn (gen_atomic_test_and_set (target, mem, GEN_INT (model))); + + return target; + #endif +} /* This function expands the legacy _sync_lock test_and_set operation which is generally an atomic exchange. Some limited targets only allow the @@ -7464,11 +7489,8 @@ /* If there are no other options, try atomic_test_and_set if the value being stored is 1. */ - if (!ret && val == const1_rtx && HAVE_atomic_test_and_set) - { - ret = gen_atomic_test_and_set (target, mem, GEN_INT (MEMMODEL_ACQUIRE)); - emit_insn (ret); - } + if (!ret && val == const1_rtx) + ret = maybe_emit_atomic_test_and_set (target, mem, MEMMODEL_ACQUIRE); return ret; } @@ -7488,16 +7510,12 @@ if (target == NULL_RTX) target = gen_reg_rtx (mode); - if (HAVE_atomic_test_and_set) - { - ret = gen_atomic_test_and_set (target, mem, GEN_INT (MEMMODEL_ACQUIRE)); - emit_insn (ret); - return ret; - } + ret = maybe_emit_atomic_test_and_set (target, mem, MEMMODEL_ACQUIRE); /* If there is no test and set, try exchange, then a compare_and_swap loop, then __sync_test_and_set. */ - ret = maybe_emit_atomic_exchange (target, mem, const1_rtx, model); + if (!ret) + ret = maybe_emit_atomic_exchange (target, mem, const1_rtx, model); if (!ret) ret = maybe_emit_compare_and_swap_exchange_loop (target, mem, const1_rtx);