From patchwork Wed May 27 11:00:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 477057 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 A908214078C for ; Wed, 27 May 2015 21:01:07 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=S5FIYfi7; 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; q=dns; s= default; b=hl9wuAGQqSffOCEKyR7yFehJqopeWjO17YRaPzd2V6B8Yh14ihOL/ rSSFKGsSnvouoAZdphCyJv+ZWrNnEZifeuo/iJ3kYSYZe90DKyBRIwefo3cGZju/ XbasGDWW+j5vDduMNRJmqBpZbka6Od80yOsaeexwFMi++vrPjb5K3c= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type:content-transfer-encoding; s=default; bh=AeGmPnS2V+5FymX4b4HLgGawPGk=; b=S5FIYfi70PSDEpCOno6lKgcfH6GB GFBb0FyljQbUcyuTJqbOViNZQ0Q9B4jw1HYjstrwunjpF4K678zEL4AG5+bp8h6H ey/pWFjvidRF7C+fy7j9Sv4ukvGpb6PH/mikoyLVVNmH/YKOhHwRMlQr6vqXMzHz 1FAUObF87KG28+Q= Received: (qmail 111964 invoked by alias); 27 May 2015 11:01:02 -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 111953 invoked by uid 89); 27 May 2015 11:01:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 27 May 2015 11:00:59 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-20.uk.mimecast.lan; Wed, 27 May 2015 12:00:56 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 27 May 2015 12:00:56 +0100 Message-ID: <5565A3E8.2090000@arm.com> Date: Wed, 27 May 2015 12:00:56 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Sandra Loosemore , GCC Patches CC: Chung-Lin Tang , Richard Earnshaw , Ramana Radhakrishnan Subject: Re: ping**3 [PATCH, ARM] Cortex-A9 MPCore volatile load workaround References: <5564C53E.8000801@codesourcery.com> In-Reply-To: <5564C53E.8000801@codesourcery.com> X-MC-Unique: luW6aqXYTlKV2HRGIOLbXg-1 X-IsSubscribed: yes Hi Sandra, Chung-Lin, A couple of comments from me, On 26/05/15 20:10, Sandra Loosemore wrote: > Chung-Lin posted this patch last year but it seems never to have been > reviewed: > > https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00714.html > > I've just re-applied and re-tested it and it still seems to be good. > Can somebody please take a look at it? > > -Sandra > +mfix-cortex-a9-volatile-hazards +Target Report Var(fix_a9_volatile_hazards) Init(0) +Avoid errata causing read-after-read hazards for concurrent volatile +accesses on Cortex-A9 MPCore processors. s/errata/erratum/ +;; Thumb-2 version allows conditional execution +(define_insn "*memory_barrier_t2" + [(set (match_operand:BLK 0 "" "") + (unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BARRIER))] + "TARGET_HAVE_MEMORY_BARRIER && TARGET_THUMB2" + { + if (TARGET_HAVE_DMB) + { + /* Note we issue a system level barrier. We should consider issuing + a inner shareabilty zone barrier here instead, ie. "DMB ISH". */ + /* ??? Differentiate based on SEQ_CST vs less strict? */ + return "dmb%?\tsy"; + } + + if (TARGET_HAVE_DMB_MCR) + return "mcr%?\tp15, 0, r0, c7, c10, 5"; + + gcc_unreachable (); + } + [(set_attr "length" "4") + (set_attr "conds" "nocond") + (set_attr "predicable" "yes")]) + This should also set the 'predicable_short_it' attribute to "no" since we don't want it to be predicated when compiling for ARMv8-A Thumb2. Consequently: + if (body == NULL_RTX) + return false; + + code = GET_CODE (body); + + if (code == SET) + { + lhs = SET_DEST (body); + rhs = SET_SRC (body); + + if (!REG_P (lhs) && GET_CODE (lhs) != SUBREG) + return false; + + if ((MEM_P (rhs) || GET_CODE (rhs) == SYMBOL_REF) + && MEM_VOLATILE_P (rhs)) + return true; + } + else + { + fmt = GET_RTX_FORMAT (code); + + for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--) + { + if (fmt[i] == 'e') + { + if (any_volatile_loads_p (XEXP (body, i))) + return true; + } + else if (fmt[i] == 'E') + for (j = 0; j < XVECLEN (body, i); j++) + if (any_volatile_loads_p (XVECEXP (body, i, j))) + return true; + } + } + + return false; +} Would it be simpler to write this using the FOR_EACH_SUBRTX infrastructure? I think it would make this function much shorter. @@ -17248,6 +17334,9 @@ arm_reorg (void) { rtx table; + if (fix_a9_volatile_hazards) + arm_cortex_a9_errata_reorg (insn); + note_invalid_constants (insn, address, true); address += get_attr_length (insn); Does the logic for adding the insn length to address need to be updated in any way since we're inserting a new instruction in the stream? The calculations here always confuse me... Thanks, Kyrill Index: testsuite/gcc.target/arm/a9-volatile-ordering-erratum-2.c =================================================================== --- testsuite/gcc.target/arm/a9-volatile-ordering-erratum-2.c (revision 0) +++ testsuite/gcc.target/arm/a9-volatile-ordering-erratum-2.c (revision 0) @@ -0,0 +1,14 @@ +/* { dg-do compile { target arm_dmb } } */ +/* { dg-options "-O2 -mthumb -mfix-cortex-a9-volatile-hazards" } */ Please add a -mno-restrict-it to the options here so that when armv8-a is the default architecture we are still allowed to conditionalise dmb. +static bool +any_volatile_loads_p (const_rtx body) +{ + int i, j; + rtx lhs, rhs; + enum rtx_code code; + const char *fmt; +