From patchwork Fri Jan 29 14:58:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 575659 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 379F3140B99 for ; Sat, 30 Jan 2016 01:58:24 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YRzxQ79r; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=HaJTRGbL2G+OPDEWpDz7Co17Gd6tX08Mro4h4hr5QWilh+unYnzvq cEtLEIZkmBs3sEFXXCuOjAjpTv9pHORvISL5/SfcVn4Rj9oePIDxZ2rjzXh7eiMM dLrsakAWidaAP4yA3aTctimSakwCQbxksSZao/EzxiVFtv1RPScRxU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=XYEnQHNByPcmlUYOYiQtcA8wIFk=; b=YRzxQ79rBJFNhF4SxxbT fliXKRUsKLLcTfVB/MEz0orH4hTwHhDIQZW7uk90mgoMEuUdLM/QUyH27J35QFIx 6GAuuCy8cGAfZbz+e1yq9FZDZFkhpzBjLDls1nMc7q+zEn+pHgUfGNd0cTxGx+Hm +z/w3q0JD4HvEuGlzOWHIzk= Received: (qmail 18991 invoked by alias); 29 Jan 2016 14:58:16 -0000 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 Received: (qmail 18968 invoked by uid 89); 29 Jan 2016 14:58:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=assess, Mask, documentations, Callers X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 29 Jan 2016 14:58:14 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 1B961ACA7 for ; Fri, 29 Jan 2016 14:58:11 +0000 (UTC) Date: Fri, 29 Jan 2016 15:58:10 +0100 From: Martin Jambor To: GCC Patches Subject: [hsa] Atomic assess memory model fixes Message-ID: <20160129145810.GB3302@virgil.suse.cz> Mail-Followup-To: GCC Patches MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes Hi, this is a followup to comments by Jakub and Richi on handling of memory models in HSA atomic operations: - I have made user-visible diagnostics lower case simple words, rather than constant identifiers. - I have added masking by MEMMODEL_BASE_MASK where appropriate. - I have made sure that warning code does not crash even when it encounters an unknown model and that it never warns multiple times. - I have fixed handling of atomic load operations which wrongly insisted on release semantics instead of acquire (apart from relaxed). - And last but not least, after looking at the respective documentations, I have convinced myself that __ATOMIC_SEQ_CST can be implemented using the HSA scacq, screl and scar memory orders, so I implemented that. Bootstrapped and tested on x86_64-linux. Since all of the above seems to be worth fixing and low risk, I am going to commit it trunk even at this stage, even though of course nothing in HSA is a regression. Thanks, Martin 2016-01-29 Martin Jambor * hsa-gen.c (get_memory_order_name): Mask with MEMMODEL_BASE_MASK. Use short lowercase names. (get_memory_order): Mask with MEMMODEL_BASE_MASK. Support MEMMODEL_CONSUME with acquire semantics and MEMMODEL_SEQ_CST with acq_rel one. Protect warning agains segfaults if get_memory_order_name returns NULL. (gen_hsa_ternary_atomic_for_builtin): Support with MEMMODEL_SEQ_CST with release semantics. Do not warn if get_memory_order already did. (gen_hsa_insns_for_call): Support with MEMMODEL_SEQ_CST with acquire semantics. Fix check for relaxed or acquire semantics. Do not warn if get_memory_order already did. --- gcc/hsa-gen.c | 59 ++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/gcc/hsa-gen.c b/gcc/hsa-gen.c index e8f80da..768c2cf 100644 --- a/gcc/hsa-gen.c +++ b/gcc/hsa-gen.c @@ -4415,20 +4415,20 @@ get_address_from_value (tree val, hsa_bb *hbb) static const char * get_memory_order_name (unsigned memmodel) { - switch (memmodel) + switch (memmodel & MEMMODEL_BASE_MASK) { case MEMMODEL_RELAXED: - return "__ATOMIC_RELAXED"; + return "relaxed"; case MEMMODEL_CONSUME: - return "__ATOMIC_CONSUME"; + return "consume"; case MEMMODEL_ACQUIRE: - return "__ATOMIC_ACQUIRE"; + return "acquire"; case MEMMODEL_RELEASE: - return "__ATOMIC_RELEASE"; + return "release"; case MEMMODEL_ACQ_REL: - return "__ATOMIC_ACQ_REL"; + return "acq_rel"; case MEMMODEL_SEQ_CST: - return "__ATOMIC_SEQ_CST"; + return "seq_cst"; default: return NULL; } @@ -4440,21 +4440,31 @@ get_memory_order_name (unsigned memmodel) static BrigMemoryOrder get_memory_order (unsigned memmodel, location_t location) { - switch (memmodel) + switch (memmodel & MEMMODEL_BASE_MASK) { case MEMMODEL_RELAXED: return BRIG_MEMORY_ORDER_RELAXED; + case MEMMODEL_CONSUME: + /* HSA does not have an equivalent, but we can use the slightly stronger + ACQUIRE. */ case MEMMODEL_ACQUIRE: return BRIG_MEMORY_ORDER_SC_ACQUIRE; case MEMMODEL_RELEASE: return BRIG_MEMORY_ORDER_SC_RELEASE; case MEMMODEL_ACQ_REL: + case MEMMODEL_SEQ_CST: + /* Callers implementing a simple load or store need to remove the release + or acquire part respectively. */ return BRIG_MEMORY_ORDER_SC_ACQUIRE_RELEASE; default: - HSA_SORRY_ATV (location, - "support for HSA does not implement memory model: %s", - get_memory_order_name (memmodel)); - return BRIG_MEMORY_ORDER_NONE; + { + const char *mmname = get_memory_order_name (memmodel); + HSA_SORRY_ATV (location, + "support for HSA does not implement the specified " + " memory model%s %s", + mmname ? ": " : "", mmname ? mmname : ""); + return BRIG_MEMORY_ORDER_NONE; + } } } @@ -4523,13 +4533,20 @@ gen_hsa_ternary_atomic_for_builtin (bool ret_orig, nops = 2; } - if (acode == BRIG_ATOMIC_ST && memorder != BRIG_MEMORY_ORDER_RELAXED - && memorder != BRIG_MEMORY_ORDER_SC_RELEASE) + if (acode == BRIG_ATOMIC_ST) { - HSA_SORRY_ATV (gimple_location (stmt), - "support for HSA does not implement memory model for " - "ATOMIC_ST: %s", get_memory_order_name (mmodel)); - return; + if (memorder == BRIG_MEMORY_ORDER_SC_ACQUIRE_RELEASE) + memorder = BRIG_MEMORY_ORDER_SC_RELEASE; + + if (memorder != BRIG_MEMORY_ORDER_RELAXED + && memorder != BRIG_MEMORY_ORDER_SC_RELEASE + && memorder != BRIG_MEMORY_ORDER_NONE) + { + HSA_SORRY_ATV (gimple_location (stmt), + "support for HSA does not implement memory model for " + "ATOMIC_ST: %s", get_memory_order_name (mmodel)); + return; + } } hsa_insn_atomic *atominsn = new hsa_insn_atomic (nops, opcode, acode, mtype, @@ -4872,8 +4889,12 @@ gen_hsa_insns_for_call (gimple *stmt, hsa_bb *hbb) BrigMemoryOrder memorder = get_memory_order (mmodel, gimple_location (stmt)); + if (memorder == BRIG_MEMORY_ORDER_SC_ACQUIRE_RELEASE) + memorder = BRIG_MEMORY_ORDER_SC_ACQUIRE; + if (memorder != BRIG_MEMORY_ORDER_RELAXED - && memorder != BRIG_MEMORY_ORDER_SC_RELEASE) + && memorder != BRIG_MEMORY_ORDER_SC_ACQUIRE + && memorder != BRIG_MEMORY_ORDER_NONE) { HSA_SORRY_ATV (gimple_location (stmt), "support for HSA does not implement "