From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31054 invoked by alias); 17 Jan 2019 14:27:48 -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 30962 invoked by uid 89); 17 Jan 2019 14:27:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=BAYES_05,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_NUMSUBJECT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=shrink-wrapping, shrinkwrapping, H*Ad:D*eu, Meanwhile X-HELO: mail-vs1-f65.google.com Received: from mail-vs1-f65.google.com (HELO mail-vs1-f65.google.com) (209.85.217.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Jan 2019 14:27:44 +0000 Received: by mail-vs1-f65.google.com with SMTP id u11so6290155vsp.11 for ; Thu, 17 Jan 2019 06:27:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=j9p4lRI3JSTYh9qE0JXbvg9N78G1VxhUfvehTN7J/dw=; b=euZIlIEjnOkcfp1KS2p4mD+zAbJ4QhgJmYRuG4Z2Q7UFKqTGPxEhDSWg9Y5/qxpLkF L8Fk5ryked4sb5y8Z7VYYz2xM3bOCnTTxb2jXrGkcpWYWuj4yp2CWb8xuh/RAVdSRMKn lU24Oav9eBJAzmDeVLDIu7+n735O3x8Wz4xYk= MIME-Version: 1.0 References: <85840089.MtehzfUrTt@tpdeb> <20190107092337.GM30353@tucnak> <87lg3vicg5.fsf@arm.com> <088aeff6-8a4c-7bad-0417-b6b3568ab3ae@redhat.com> In-Reply-To: <088aeff6-8a4c-7bad-0417-b6b3568ab3ae@redhat.com> From: Christophe Lyon Date: Thu, 17 Jan 2019 14:27:00 -0000 Message-ID: Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807 To: Jeff Law Cc: Bernd Edlinger , Jakub Jelinek , Dimitar Dimitrov , Segher Boessenkool , Thomas Preudhomme , "gcc-patches@gcc.gnu.org" , Richard Sandiford Content-Type: multipart/mixed; boundary="00000000000088076d057fa832ed" X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg01008.txt.bz2 --00000000000088076d057fa832ed Content-Type: text/plain; charset="UTF-8" Content-length: 4372 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 --00000000000088076d057fa832ed Content-Type: text/plain; charset="US-ASCII"; name="pr77904.chlog.txt" Content-Disposition: attachment; filename="pr77904.chlog.txt" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_jr0plqrj0 Content-length: 163 MjAxOS0wMS0xNyAgQ2hyaXN0b3BoZSBMeW9uICA8Y2hyaXN0b3BoZS5seW9u QGxpbmFyby5vcmc+CgoJKiBnY2MudGFyZ2V0L2FybS9wcjc3OTA0LmM6IEFk ZCBkZy13YXJuaW5nIGZvciBzcCBjbG9iYmVyLgoK --00000000000088076d057fa832ed Content-Type: text/plain; charset="US-ASCII"; name="pr77904.patch.txt" Content-Disposition: attachment; filename="pr77904.patch.txt" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_jr0plqsk1 Content-length: 639 ZGlmZiAtLWdpdCBhL2djYy90ZXN0c3VpdGUvZ2NjLnRhcmdldC9hcm0vcHI3 NzkwNC5jIGIvZ2NjL3Rlc3RzdWl0ZS9nY2MudGFyZ2V0L2FybS9wcjc3OTA0 LmMKaW5kZXggNzY3MjhjMC4uZjI3OWVjOCAxMDA2NDQKLS0tIGEvZ2NjL3Rl c3RzdWl0ZS9nY2MudGFyZ2V0L2FybS9wcjc3OTA0LmMKKysrIGIvZ2NjL3Rl c3RzdWl0ZS9nY2MudGFyZ2V0L2FybS9wcjc3OTA0LmMKQEAgLTQsNyArNCw4 IEBACiBfX2F0dHJpYnV0ZV9fICgobm9pbmxpbmUsIG5vY2xvbmUpKSB2b2lk CiBjbG9iYmVyX3NwICh2b2lkKQogewotICBfX2FzbSB2b2xhdGlsZSAoIiIg OiA6IDogInNwIik7CisgIF9fYXNtIHZvbGF0aWxlICgiIiA6IDogOiAic3Ai KTsgLyogeyBkZy13YXJuaW5nICJsaXN0aW5nIHRoZSBzdGFjayBwb2ludGVy IHJlZ2lzdGVyICdzcCcgaW4gYSBjbG9iYmVyIGxpc3QgaXMgZGVwcmVjYXRl ZCIgfSAqLworCiB9CiAKIGludAo= --00000000000088076d057fa832ed--