From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24752 invoked by alias); 7 Jan 2013 16:52:29 -0000 Received: (qmail 24744 invoked by uid 22791); 7 Jan 2013 16:52:28 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_VZ X-Spam-Check-By: sourceware.org Received: from mail-ob0-f176.google.com (HELO mail-ob0-f176.google.com) (209.85.214.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Jan 2013 16:52:24 +0000 Received: by mail-ob0-f176.google.com with SMTP id un3so17285887obb.7 for ; Mon, 07 Jan 2013 08:52:23 -0800 (PST) MIME-Version: 1.0 Received: by 10.60.30.231 with SMTP id v7mr32449027oeh.22.1357577543517; Mon, 07 Jan 2013 08:52:23 -0800 (PST) Received: by 10.182.153.201 with HTTP; Mon, 7 Jan 2013 08:52:23 -0800 (PST) In-Reply-To: <20130106162251.GX7269@tucnak.redhat.com> References: <20130106162251.GX7269@tucnak.redhat.com> Date: Mon, 07 Jan 2013 16:52:00 -0000 Message-ID: Subject: Re: [PATCH, dataflow]: Fix PR55845, 454.calculix miscompares on x86 AVX due to movement of vzeroupper From: Uros Bizjak To: Jakub Jelinek Cc: Paolo Bonzini , Richard Henderson , gcc-patches@gcc.gnu.org, Vladimir Yakovlev , "Kumar, Venkataramanan" , Eric Botcazou Content-Type: text/plain; charset=ISO-8859-1 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 X-SW-Source: 2013-01/txt/msg00330.txt.bz2 On Sun, Jan 6, 2013 at 5:22 PM, Jakub Jelinek wrote: >> --- df-problems.c (revision 194945) >> +++ df-problems.c (working copy) >> @@ -3916,6 +3916,10 @@ can_move_insns_across (rtx from, rtx to, rtx acros >> break; >> if (NONDEBUG_INSN_P (insn)) >> { >> + /* Do not move unspec_volatile insns. */ >> + if (GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE) >> + break; >> + > > Shouldn't UNSPEC_VOLATILE be handled similarly in the across_from .. > across_to loop? Both UNSPEC_VOLATILE and volatile asm are handled there > just with > trapping_insns_in_across |= may_trap_p (PATTERN (insn)); > but your new change doesn't prevent moving just trapping insns across > UNSPEC_VOLATILE, but any insns whatsoever. So supposedly for UNSPEC_VOLATILE > the first loop should just return false; (or fail = 1; ?). > For asm volatile I guess the code is fine as is, it must always describe > what exactly it modifies, so supposedly non-trapping insns can be moved > across asm volatile. > >> if (may_trap_or_fault_p (PATTERN (insn)) >> && (trapping_insns_in_across || other_branch_live != NULL)) >> break; > > You could do the check only for may_trap_or_fault_p, all UNSPEC_VOLATILE > may trap. > > BTW, can't UNSPEC_VOLATILE be embedded deeply in the pattern? > So volatile_insn_p (insn) && asm_noperands (PATTERN (insn)) == -1? > But perhaps you want to treat that way only UNSPEC_VOLATILE directly in the > pattern and all other UNSPEC_VOLATILE insns must describe in detail what > exactly they are changing? This really needs to be better documented. TBH, I'm not that familiar with the RTL infrastructure enough to answer these questions. While I can spend some time on this problem, and probably waste quite some reviewer's time, the problem is not that trivial as I hoped to be, so I would kindly ask someone with better understanding of this part of the compiler for the proper solution. Uros.