From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23096 invoked by alias); 12 Nov 2012 08:28:35 -0000 Received: (qmail 23085 invoked by uid 22791); 12 Nov 2012 08:28:32 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 12 Nov 2012 08:28:25 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 78ECACB0C86; Mon, 12 Nov 2012 09:28:26 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wyFOLogx-eAO; Mon, 12 Nov 2012 09:28:26 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id DC316CB052B; Mon, 12 Nov 2012 09:28:25 +0100 (CET) From: Eric Botcazou To: Hans-Peter Nilsson Cc: gcc-patches@gcc.gnu.org Subject: Re: [RFA:] fix PR55030, wrong code from __builtin_setjmp_receiver Date: Mon, 12 Nov 2012 08:28:00 -0000 Message-ID: <13011180.NBQR3vZSIa@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.16-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: 2012-11/txt/msg00877.txt.bz2 > This is a target-specific blockage insn, but with the general form > found in all targets defining it. The default blockage is an empty > asm-volatile, which is what cse_insn recognized. The blockage insn is > there to "prevent scheduling" of the critical insns and register > values. It's almost obvious that an unspec_volatile should have that > effect "too"; at least that's intended by the code in > expand_builtin_setjmp_receiver. Luckily (or unluckily regarding the > presence of the bug) *this* cse code is not run post-frame-layout > (post-reload-cse does not use this code), or it seems people would > soon notice register values used from the wrong side of the blockage, > considering its critical use at the restore of the stack-pointer. > As mentioned in the previous patch, > , clobbering > the frame-pointer (and then using it) does not seem the logical > alternative to the patch below; the blockage insn should just do its job. Agreed. > I updated comments and documentation so it's more apparent that > register uses should also not be moved across the blockage; see the > patched comments. > > Tested native x86_64-unknown-linux-gnu w./wo. -m32 and before/after > 192677. Ok to commit? > > gcc: > PR middle-end/55030 > * builtins.c (expand_builtin_setjmp_receiver): Update comment > regarding purpose of blockage. > * emit-rtl.c [!HAVE_blockage] (gen_blockage): Similarly for > the head comment. > * doc/md.texi (blockage): Update similarly. Change wording to > require one of two forms, rather than implying a wider choice. > * cse.c (cse_insn): Where checking for blocking insns, treat > UNSPEC_VOLATILE as blocking, besides volatile ASM. That's fine on principle, but there is a predicate for this (volatile_insn_p) so I think we should use it here. Moreover, cselib_process_insn has the same check so we should adjust it as well, which in turn means that dse.c:scan_insn should probably be adjusted too. OK with these changes, thanks. -- Eric Botcazou