From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39643 invoked by alias); 17 Jan 2020 02:03:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 39608 invoked by uid 89); 17 Jan 2020 02:03:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_MANYTO,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-HELO:sk:mail-vs, stxr X-HELO: mail-vs1-f54.google.com Received: from mail-vs1-f54.google.com (HELO mail-vs1-f54.google.com) (209.85.217.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Jan 2020 02:03:48 +0000 Received: by mail-vs1-f54.google.com with SMTP id g15so13983686vsf.1 for ; Thu, 16 Jan 2020 18:03:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=Lfw/5anUD/9/kW99EBWy1QflNshS1eOvulVuk1sx6Bc=; b=MV2zh4e7tMhhKSGP3PcqgZ8Y+F99aV2bYJEHFIdIs9usS59titHH1YWO0N3K+0S7VW PNrTvqYx9SpIm+Egf5BeNB1P4G0KH2KRR3hWS5oCDgzOZqulRgf90AEiEm+5AMBjLDmc 0N6LnOJ7ncP3SGUln1vzzJW8OSq+PxrHxWh3radmyAXV3NO8kE/R0laXIRTMYVvFggya RQb69+iywhHFcha0ow5X5thEJ3BFKNljTXv29QQ1VWCPOW5A+nuAcM8KntRnIiYAyaG7 AaKB5CVjD6rrGnFgvdq9Nuq2e5HvU8ADPqPwVZnL/1izSCMCYL7959bfm1ZmyUb2eVYB Em0Q== MIME-Version: 1.0 References: In-Reply-To: From: Andrew Pinski Date: Fri, 17 Jan 2020 03:16:00 -0000 Message-ID: Subject: Re: [PATCH][AArch64] Fix shrinkwrapping interactions with atomics (PR92692) To: Wilco Dijkstra , GCC Patches , Andrew Pinski , Kyrylo Tkachov , Richard Earnshaw , richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg01006.txt.bz2 On Thu, Jan 16, 2020 at 5:51 PM Andrew Pinski wrote: > > On Thu, Jan 16, 2020 at 5:14 AM Richard Sandiford > wrote: > > > > Wilco Dijkstra writes: > > > The separate shrinkwrapping pass may insert stores in the middle > > > of atomics loops which can cause issues on some implementations. > > > Avoid this by delaying splitting of atomic patterns until after > > > prolog/epilog generation. > > > > > > Bootstrap completed, no test regressions on AArch64. > > > > > > Andrew, can you verify this fixes the failure you were getting? > > > > > > ChangeLog: > > > 2020-01-16 Wilco Dijkstra > > > > > > PR target/92692 > > > * config/aarch64/aarch64.c (aarch64_split_compare_and_swap) > > > Add assert to ensure prolog has been emitted. > > > (aarch64_split_atomic_op): Likewise. > > > * config/aarch64/atomics.md (aarch64_compare_and_swap) > > > Use epilogue_completed rather than reload_completed. > > > (aarch64_atomic_exchange): Likewise. > > > (aarch64_atomic_): Likewise. > > > (atomic_nand): Likewise. > > > (aarch64_atomic_fetch_): Likewise. > > > (atomic_fetch_nand): Likewise. > > > (aarch64_atomic__fetch): Likewise. > > > (atomic_nand_fetch): Likewise. > > > > OK if Andrew confirms it works, thanks. > > Yes this fixes the issue for me. Here is the new assembly showing it worked: d390: f9000bf3 str x19, [sp, #16] d394: 885ffdc8 ldaxr w8, [x14] d398: 6b01011f cmp w8, w1 d39c: 54000061 b.ne d3a8 // b.any d3a0: 88137dc5 stxr w19, w5, [x14] Thanks, Andrew Pinski > > Thanks, > Andrew > > > > > Richard > > > > > --- > > > > > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > > > index ac89cc1f9c938455d33d8850d9ebfc0473cb73dc..cd9d813f2ac4990971f6435fdb28b0f94ae10309 100644 > > > --- a/gcc/config/aarch64/aarch64.c > > > +++ b/gcc/config/aarch64/aarch64.c > > > @@ -18375,6 +18375,9 @@ aarch64_emit_post_barrier (enum memmodel model) > > > void > > > aarch64_split_compare_and_swap (rtx operands[]) > > > { > > > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */ > > > + gcc_assert (epilogue_completed); > > > + > > > rtx rval, mem, oldval, newval, scratch, x, model_rtx; > > > machine_mode mode; > > > bool is_weak; > > > @@ -18469,6 +18472,9 @@ void > > > aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem, > > > rtx value, rtx model_rtx, rtx cond) > > > { > > > + /* Split after prolog/epilog to avoid interactions with shrinkwrapping. */ > > > + gcc_assert (epilogue_completed); > > > + > > > machine_mode mode = GET_MODE (mem); > > > machine_mode wmode = (mode == DImode ? DImode : SImode); > > > const enum memmodel model = memmodel_from_int (INTVAL (model_rtx)); > > > diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md > > > index c2bcabd0c3c2627b7222dcbc1af9c2e6b7ce6a76..996947799b5ef8445e9786b94e1ce62fd16e5b5c 100644 > > > --- a/gcc/config/aarch64/atomics.md > > > +++ b/gcc/config/aarch64/atomics.md > > > @@ -56,7 +56,7 @@ (define_insn_and_split "@aarch64_compare_and_swap" > > > (clobber (match_scratch:SI 7 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_compare_and_swap (operands); > > > @@ -80,7 +80,7 @@ (define_insn_and_split "@aarch64_compare_and_swap" > > > (clobber (match_scratch:SI 7 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_compare_and_swap (operands); > > > @@ -104,7 +104,7 @@ (define_insn_and_split "@aarch64_compare_and_swap" > > > (clobber (match_scratch:SI 7 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_compare_and_swap (operands); > > > @@ -223,7 +223,7 @@ (define_insn_and_split "aarch64_atomic_exchange" > > > (clobber (match_scratch:SI 4 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (SET, operands[0], NULL, operands[1], > > > @@ -344,7 +344,7 @@ (define_insn_and_split "aarch64_atomic_" > > > (clobber (match_scratch:SI 4 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (, NULL, operands[3], operands[0], > > > @@ -400,7 +400,7 @@ (define_insn_and_split "atomic_nand" > > > (clobber (match_scratch:SI 4 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (NOT, NULL, operands[3], operands[0], > > > @@ -504,7 +504,7 @@ (define_insn_and_split "aarch64_atomic_fetch_" > > > (clobber (match_scratch:SI 5 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (, operands[0], operands[4], operands[1], > > > @@ -551,7 +551,7 @@ (define_insn_and_split "atomic_fetch_nand" > > > (clobber (match_scratch:SI 5 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (NOT, operands[0], operands[4], operands[1], > > > @@ -604,7 +604,7 @@ (define_insn_and_split "aarch64_atomic__fetch" > > > (clobber (match_scratch:SI 4 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (, NULL, operands[0], operands[1], > > > @@ -628,7 +628,7 @@ (define_insn_and_split "atomic_nand_fetch" > > > (clobber (match_scratch:SI 4 "=&r"))] > > > "" > > > "#" > > > - "&& reload_completed" > > > + "&& epilogue_completed" > > > [(const_int 0)] > > > { > > > aarch64_split_atomic_op (NOT, NULL, operands[0], operands[1],