From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38614 invoked by alias); 5 Feb 2020 10:47:08 -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 38563 invoked by uid 89); 5 Feb 2020 10:47:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*RU:209.85.166.66, HX-Spam-Relays-External:209.85.166.66, H*r:ip*209.85.166.66, peephole2 X-HELO: mail-io1-f66.google.com Received: from mail-io1-f66.google.com (HELO mail-io1-f66.google.com) (209.85.166.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Feb 2020 10:47:04 +0000 Received: by mail-io1-f66.google.com with SMTP id z8so1620418ioh.0 for ; Wed, 05 Feb 2020 02:47:04 -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 :cc; bh=L5V+qTUj1PYNNP26b7pmeRYmxOeSjIMKsw5zXYhbQS4=; b=okdRIysWFlMYp+vTQ87+zKt2lMX8kXFpAM8aQgcs+pab8y4LPs193fg9UPXOzJg5Kw Y2BvQvgBUQ4q/5JNrimEswPG57um2UKTREGDTrX3Wk4T1vd7SmUQ68LgdkZF7nr8ybE9 LeMLL8LNfnEN/xYrTi8yveIZbmIdJB1h6/T2xotMASPvkJnZWy8X1tmgksmTSXN/F/rA CewkuZE+9jHdfcGO8LbwITw/HhS0uRFGnzFBkhMxPtuX5q4OVWPSGCgKFh36LUVf5V0s z2JVvIM2KEn0DH5G6PRwVH26o+BrdOenZxzXbwxjiT3bbTpK9LfTkjTyX6zjpoRr+Rs5 UjVA== MIME-Version: 1.0 References: <20200204093921.GN17695@tucnak> <20200204110521.GO17695@tucnak> <20200204123040.GQ17695@tucnak> <20200204131257.GR17695@tucnak> <20200205095804.GW17695@tucnak> In-Reply-To: <20200205095804.GW17695@tucnak> From: Uros Bizjak Date: Wed, 05 Feb 2020 10:47:00 -0000 Message-ID: Subject: Re: [PATCH] i386: Omit clobbers from vzeroupper until final [PR92190] To: Jakub Jelinek Cc: Richard Sandiford , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2020-02/txt/msg00252.txt.bz2 On Wed, Feb 5, 2020 at 11:05 AM Jakub Jelinek wrote: > > On Tue, Feb 04, 2020 at 02:15:04PM +0100, Uros Bizjak wrote: > > On Tue, Feb 4, 2020 at 2:13 PM Jakub Jelinek wrote: > > > > > > On Tue, Feb 04, 2020 at 01:38:51PM +0100, Uros Bizjak wrote: > > > > As Richard advised, let's put this safety stuff back. Usually, in > > > > i386.md, these kind of splitters are implemented as two patterns, one > > > > (define_insn_and_split) having "#", and the other (define_insn) with a > > > > real insn. My opinion is, that this separation avoids confusion as > > > > much as possible. > > > > > > Okay. So like this if it passes bootstrap/regtest then? > > > > Yes. > > > > > 2020-02-04 Jakub Jelinek > > > > > > PR target/92190 > > > * config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): Only > > > include sets and not clobbers in the vzeroupper pattern. > > > * config/i386/sse.md (*avx_vzeroupper): Require in insn condition that > > > the parallel has 17 (64-bit) or 9 (32-bit) elts. > > > (*avx_vzeroupper_1): New define_insn_and_split. > > > > > > * gcc.target/i386/pr92190.c: New test. > > > > OK. > > Unfortunately, it breaks > +FAIL: gcc.target/i386/avx-2.c (internal compiler error) > +FAIL: gcc.target/i386/avx-2.c (test for excess errors) > +FAIL: gcc.target/i386/avx-vzeroupper-10.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-10.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-10.c scan-assembler-times avx_vzeroupper 3 > +FAIL: gcc.target/i386/avx-vzeroupper-11.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-11.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-11.c scan-assembler-times \\\\*avx_vzeroall 1 > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-11.c scan-assembler-times avx_vzeroupper 3 > +FAIL: gcc.target/i386/avx-vzeroupper-12.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-12.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-12.c scan-assembler-times \\\\*avx_vzeroall 1 > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-12.c scan-assembler-times avx_vzeroupper 4 > +FAIL: gcc.target/i386/avx-vzeroupper-5.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-5.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-5.c scan-assembler-times avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-7.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-7.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-7.c scan-assembler-times avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-8.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-8.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-8.c scan-assembler-times avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-9.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-9.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-9.c scan-assembler-times avx_vzeroupper 4 > +FAIL: gcc.target/i386/sse-14.c (internal compiler error) > +FAIL: gcc.target/i386/sse-14.c (test for excess errors) > +FAIL: gcc.target/i386/sse-22.c (internal compiler error) > +FAIL: gcc.target/i386/sse-22.c (test for excess errors) > +FAIL: gcc.target/i386/sse-22a.c (internal compiler error) > +FAIL: gcc.target/i386/sse-22a.c (test for excess errors) > > The problem is that x86 is the only target that defines HAVE_ATTR_length and > doesn't schedule any splitting pass at -O0 after pro_and_epilogue. > > So, either we go back to handling this during vzeroupper output > (unconditionally, rather than flag_ipa_ra guarded), or we need to tweak the > split* passes for x86. > > Seems there are 5 split passes, split1 is run unconditionally before reload, > split2 is run for optimize > 0 or STACK_REGS (x86) after ra but before > epilogue_completed, split3 is run before regstack for STACK_REGS and > optimize and -fno-schedule-insns2, split4 is run before sched2 if sched2 is > run and split5 is run before shorten_branches if HAVE_ATTR_length and not > STACK_REGS. > > Attached are 3 possible incremental patches for recog.c, all of them fix > all the above regressions, but haven't fully bootstrapped/regtested any of > them yet. My preference would be the last one, which for -O0 and x86 > disables split2 and enables split3, as it doesn't add any extra passes. > The first one just enables split3 for -O0 on x86, the second one enables > split5 for -O0 on x86. Please note that in i386.md we expect that the check for "epilogue_completed" means split4 point. There are some places with: "TARGET_64BIT && ((optimize > 0 && flag_peephole2) ? epilogue_completed : reload_completed) so for flag_peephole2, we split after peephole2 pass was performed. Apparently, we already hit this problem in the past, so the check for "optimize > 0" was added to solve -O0 -fpeephole2 combo. Another one is with && TARGET_SSE_PARTIAL_REG_DEPENDENCY && epilogue_completed where we assume that allocated registers won't change anymore when breaking SSE partial register stall. I think we should just enable split4 also for -O0. This would also allow us to remove the "optimize > 0" check above and allow us to generate a bit more optimal code even with -O0 for TARGET_SSE_PARTIAL_REG_DEPENDENCY and TARGET_AVOID_FALSE_DEP_FOR_BMI. Uros. Uros. > > Jakub