From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82328 invoked by alias); 18 Jan 2019 09:49:13 -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 81839 invoked by uid 89); 18 Jan 2019 09:48:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,KAM_NUMSUBJECT,SPF_PASS autolearn=no version=3.3.2 spammy=Hx-languages-length:4599, noreturn, submission X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 18 Jan 2019 09:48:36 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CA91980D; Fri, 18 Jan 2019 01:48:24 -0800 (PST) Received: from localhost (unknown [10.32.98.35]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 456AD3F5C1; Fri, 18 Jan 2019 01:48:23 -0800 (PST) From: Richard Sandiford To: Christophe Lyon Mail-Followup-To: Christophe Lyon ,Jeff Law , Bernd Edlinger , Jakub Jelinek , Dimitar Dimitrov , Segher Boessenkool , Thomas Preudhomme , "gcc-patches\@gcc.gnu.org" , richard.sandiford@arm.com Cc: Jeff Law , Bernd Edlinger , Jakub Jelinek , Dimitar Dimitrov , Segher Boessenkool , Thomas Preudhomme , "gcc-patches\@gcc.gnu.org" Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807 References: <85840089.MtehzfUrTt@tpdeb> <20190107092337.GM30353@tucnak> <87lg3vicg5.fsf@arm.com> <088aeff6-8a4c-7bad-0417-b6b3568ab3ae@redhat.com> Date: Fri, 18 Jan 2019 09:49:00 -0000 In-Reply-To: (Christophe Lyon's message of "Thu, 17 Jan 2019 15:27:30 +0100") Message-ID: <87a7jyuwii.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2019-01/txt/msg01057.txt.bz2 Christophe Lyon writes: > On Fri, 11 Jan 2019 at 23:59, Jeff Law wrote: >> >> 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 > > > Hi, > > I originally complained because the arm test for pr77904.c was failing. > Since Richard's change that test emits a warning rather than an error, > but still fails. This small patch adds the missing dg-warning. > > OK? > > Thanks, > > Christophe > > 2019-01-17 Christophe Lyon > > * gcc.target/arm/pr77904.c: Add dg-warning for sp clobber. OK, thanks. Richard