From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68273 invoked by alias); 11 Jan 2019 22:59:05 -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 68264 invoked by uid 89); 11 Jan 2019 22:59:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KAM_MANYTO,KAM_NUMSUBJECT,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=submission, 52813, Bernd, H*f:sk:DB7PR07 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jan 2019 22:59:02 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B34B037E60; Fri, 11 Jan 2019 22:59:01 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-2.rdu2.redhat.com [10.10.112.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id CB5005D6B3; Fri, 11 Jan 2019 22:58:56 +0000 (UTC) Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807 To: Bernd Edlinger , Jakub Jelinek , Dimitar Dimitrov , Segher Boessenkool , Christophe Lyon , Thomas Preudhomme , "gcc-patches@gcc.gnu.org" , richard.sandiford@arm.com References: <85840089.MtehzfUrTt@tpdeb> <20190107092337.GM30353@tucnak> <87lg3vicg5.fsf@arm.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <088aeff6-8a4c-7bad-0417-b6b3568ab3ae@redhat.com> Date: Fri, 11 Jan 2019 22:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <87lg3vicg5.fsf@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00677.txt.bz2 On 1/8/19 5:03 AM, Richard Sandiford wrote: > Bernd Edlinger writes: >> On 1/7/19 10:23 AM, Jakub Jelinek wrote: >>> On Sun, Dec 16, 2018 at 06:13:57PM +0200, Dimitar Dimitrov wrote: >>>> - /* Clobbering the STACK POINTER register is an error. */ >>>> + /* Clobbered STACK POINTER register is not saved/restored by GCC, >>>> + which is often unexpected by users. See PR52813. */ >>>> if (overlaps_hard_reg_set_p (regset, Pmode, STACK_POINTER_REGNUM)) >>>> { >>>> - error ("Stack Pointer register clobbered by %qs in %", regname); >>>> + warning (0, "Stack Pointer register clobbered by %qs in %", >>>> + regname); >>>> + warning (0, "GCC has always ignored Stack Pointer % clobbers"); >>> >>> Why do we write Stack Pointer rather than stack pointer? That is really >>> weird. The second warning would be a note based on the first one, i.e. >>> if (warning ()) note (); >>> and better have some -W* option to silence the warning. >>> >> >> Yes, thanks for this suggestion. >> >> Meanwhile I found out, that the stack clobber has only been ignored up to >> gcc-5 (at least with lra targets, not really sure about reload targets). >> From gcc-6 on, with the exception of PR arm/77904 which was a regression due >> to the underlying lra change, but fixed later, and back-ported to gcc-6.3.0, >> this works for all targets I tried so far. >> >> To me, it starts to look like a rather unique and useful feature, that I would >> like to keep working. > > Not sure what you mean by "unique". But forcing a frame is a bit of > a slippery concept. Force it where? For the asm only, or the whole > function? This depends on optimisation and hasn't been consistent > across GCC versions, since it depends on the shrink-wrapping > optimisation. (There was a similar controversy a while ago about > to what extent -fno-omit-frame-pointer should "force a frame".) > > The effect on the redzone seems like something that should be specified > explicitly rather than as an (accidental?) side effect of listing the > sp in the clobber list. Maybe this would be another use for the "asm > attributes" proposal. "noreturn" was another attribute suggested on > IRC yesterday. > > But either way, the general feeling seems to be that going straight to a > hard error is too harsh, since there's quite a bit of existing code that > has the clobber. This patch implements the compromise discussed on IRC > yesterday of making it a -Wdeprecated warning instead. > > Tested on x86_64-linux-gnu and aarch64-linux-gnu. OK to install? > > Richard > > Dimitar: sorry the run-around on this patch, and thanks for the > submission. It looks from all the controversy like it was a > long-festering PR for a reason. :-/ > > > 2019-01-07 Richard Sandiford > > gcc/ > PR inline-asm/52813 > * doc/extend.texi: Document that listing the stack pointer in the > clobber list of an asm is a deprecated feature. > * common.opt (Wdeprecated): Moved from c-family/c.opt. > * cfgexpand.c (asm_clobber_reg_is_valid): Issue a -Wdeprecated > warning instead of an error for clobbers of the stack pointer. > Add a note explaining why. > > gcc/c-family/ > PR inline-asm/52813 > * c.opt (Wdeprecated): Move documentation and variable to common.opt. > > gcc/d/ > PR inline-asm/52813 > * lang.opt (Wdeprecated): Reference common.opt instead of c.opt. > > gcc/testsuite/ > PR inline-asm/52813 > * gcc.target/i386/pr52813.c (test1): Turn the diagnostic into a > -Wdeprecated warning and expect a following note:. OK. FWIW the number of packages affected in Fedora was in single digits, some of which have already been fixed. But if folks want to go with a deprecated warning instead of straight to an error, I won't complain. jeff