From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35757 invoked by alias); 12 Dec 2018 15:35: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 35742 invoked by uid 89); 12 Dec 2018 15:35:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=2.6 required=5.0 tests=BAYES_00,BODY_8BITS,GARBLED_BODY,KAM_NUMSUBJECT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy=8:=d0=b4, 8:=d0=b5?= X-HELO: mail-it1-f193.google.com Received: from mail-it1-f193.google.com (HELO mail-it1-f193.google.com) (209.85.166.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Dec 2018 15:35:44 +0000 Received: by mail-it1-f193.google.com with SMTP id h65so10414360ith.3 for ; Wed, 12 Dec 2018 07:35: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:content-transfer-encoding; bh=DRa76Yw808vn2U2LNZQ4mBqwQ+nyrDsQ5v+HiiqH1LY=; b=AA2UH4QV0Ink4y6yRPMM8soP8GXkb6jWsnwwzfQqwhjT2xBjn5Dzz8WhXM5cwPbJY4 ennxbGKQqIm+bUaZTV+iGvUc91ANmMVrm8d5gufxo/oqSlUUQMV3VDMiTCEY2NJoJlRX auipNHq6MQVOCHkgvBjvkJ6WgyDK7HWZdFN2c= MIME-Version: 1.0 References: <20181209100856.14051-1-dimitar@dinux.eu> <87woohsk32.fsf@arm.com> <2807771.CbC4dySGB1@tpdeb> <87woog9i32.fsf@arm.com> In-Reply-To: From: Thomas Preudhomme Date: Wed, 12 Dec 2018 15:35:00 -0000 Message-ID: Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807 To: Christophe Lyon Cc: dimitar@dinux.eu, gcc-patches@gcc.gnu.org, richard.sandiford@arm.com, "Thomas Preud'homme" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00825.txt.bz2 [resending from the right address] Hi Christophe, Why not simply: "Clobber of unsupported" with an accompanying change of the documentation to state the extra bit you wanted to put in that error message? Perhaps even add a reference to the section of the documentation in the error message. Best regards, Thomas On Wed, 12 Dec 2018 at 15:13, Christophe Lyon wrote: > > On Wed, 12 Dec 2018 at 14:19, Christophe Lyon > wrote: > > > > On Wed, 12 Dec 2018 at 12:21, Thomas Preudhomme > > wrote: > > > > > > So my understanding is that the original code (CMSIS library) used to > > > clobber sp because the asm statement was actually changing the sp. > > > That in turn led GCC to try to save and restore sp which is not what > > > CMSIS was expecting to happen. Changing sp without clobber as done now > > > is probably the right solution and r242693 can be reverted. That will > > > remove the failing test. > > > > > > > OK, I read PR52813 too, but I'm not sure to fully understand the new st= atus. > > My understanding is that since this patch was committed, if an asm stat= ement > > clobbers sp, it is now allowed to actually declare it as clobber (this = patch > > generates an error in such a case). > > So the user is now expected to lie to the compiler when writing to > > this kind of register (sp, pic register), by not declaring it as "clobb= er"? > > > > I'm attaching a small patch which adds a more verbose error message > along the lines of what I understand of the current status. > I'm pretty sure I got (at least) the formatting wrong :) > > Christophe > > > > > > Best regards, > > > > > > Thomas > > > On Wed, 12 Dec 2018 at 10:30, Thomas Preudhomme > > > wrote: > > > > > > > > Hi Christophe, > > > > > > > > That PR was about a bug occuring when sp was clobbered so if it can= not > > > > be clobbered anymore the whole commit (r242693) can be removed. Let= me > > > > check the original code that lead to the PR why it's clobbering sp > > > > though. > > > > > > > > Best regards, > > > > > > > > Thomas > > > > On Wed, 12 Dec 2018 at 09:43, Christophe Lyon > > > > wrote: > > > > > > > > > > On Tue, 11 Dec 2018 at 16:52, Richard Sandiford > > > > > wrote: > > > > > > > > > > > > Dimitar Dimitrov writes: > > > > > > > On =D0=BF=D0=BE=D0=BD=D0=B5=D0=B4=D0=B5=D0=BB=D0=BD=D0=B8=D0= =BA, 10 =D0=B4=D0=B5=D0=BA=D0=B5=D0=BC=D0=B2=D1=80=D0=B8 2018 =D0=B3. 11:21= :53 EET Richard Sandiford wrote: > > > > > > >> Dimitar Dimitrov writes: > > > > > > >> > I have tested this fix on x86_64 host, and found no regres= sion in the C > > > > > > >> > and C++ testsuites. I'm marking this patch as RFC simply = because I don't > > > > > > >> > have experience with other architectures, and I don't have= a setup to > > > > > > >> > test all architectures supported by GCC. > > > > > > >> > > > > > > > >> > gcc/ChangeLog: > > > > > > >> > > > > > > > >> > 2018-12-07 Dimitar Dimitrov > > > > > > >> > > > > > > > >> > * cfgexpand.c (asm_clobber_reg_is_valid): Also produce > > > > > > >> > error when stack pointer is clobbered. > > > > > > >> > (expand_asm_stmt): Refactor clobber check in separate f= unction. > > > > > > >> > > > > > > > >> > gcc/testsuite/ChangeLog: > > > > > > >> > > > > > > > >> > 2018-12-07 Dimitar Dimitrov > > > > > > >> > > > > > > > >> > * gcc.target/i386/pr52813.c: New test. > > > > > > >> > > > > > > > >> > Signed-off-by: Dimitar Dimitrov > > > > > > >> > > > > > > >> LGTM. Do you have a copyright assignment on file? 'Fraid t= his is > > > > > > >> probably big enough to need one. > > > > > > > Yes, I have copyright assignment. > > > > > > > > > > > > OK, great. I went ahead and applied the patch. > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > This patch introduces a regression on arm: > > > > > FAIL: gcc.target/arm/pr77904.c (test for excess errors) > > > > > Excess errors: > > > > > /gcc/testsuite/gcc.target/arm/pr77904.c:7:3: error: Stack Pointer > > > > > register clobbered by 'sp' in 'asm' > > > > > > > > > > Indeed the testcase has an explicit: > > > > > __asm volatile ("" : : : "sp"); > > > > > which is now rejected. > > > > > > > > > > Thomas, is that mandatory to test your code to fix pr77904? > > > > > > > > > > Thanks, > > > > > > > > > > Christophe > > > > > > > > > > > Thanks, > > > > > > Richard