From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83434 invoked by alias); 31 Dec 2018 17:17:33 -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 83425 invoked by uid 89); 31 Dec 2018 17:17:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=saving, chosen, sk:thomas, sk:thomas. X-HELO: mail-it1-f171.google.com Received: from mail-it1-f171.google.com (HELO mail-it1-f171.google.com) (209.85.166.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 31 Dec 2018 17:17:27 +0000 Received: by mail-it1-f171.google.com with SMTP id i145so36314072ita.4 for ; Mon, 31 Dec 2018 09:17:27 -0800 (PST) MIME-Version: 1.0 Received: by 2002:ac0:aed2:0:0:0:0:0 with HTTP; Mon, 31 Dec 2018 09:17:25 -0800 (PST) In-Reply-To: References: From: Thomas Preudhomme Date: Mon, 31 Dec 2018 19:14:00 -0000 Message-ID: Subject: Re: [PATCH, ARM] Fix PR77904 testcase failure To: "Richard Earnshaw (lists)" Cc: "kyrylo.tkachov@foss.arm.com" , Ramana Radhakrishnan , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-12/txt/msg01799.txt.bz2 Hi Richard, On Thursday, 20 December 2018, Richard Earnshaw (lists) < Richard.Earnshaw@arm.com> wrote: > On 14/12/2018 23:28, Thomas Preudhomme wrote: >> Hi, >> >> Commit r242693 forced fp to be saved/restored when needed due to an >> instance of GCC using fp as a scratch register to save sp while it's >> being clobbered by an inline asm. The normal path in >> thumb1_compute_save_reg_mask saving callee-saved registers which are >> live in the function does not work in that case because fp is chosen to >> hold sp after that function is called. >> >> Since clobbering sp is now errored out by the compiler and this was the >> only case reported where fp was live but not marked as such when >> thumb1_compute_save_reg_mask is called, I believe the whole commit >> r242693 should be reverted. >> >> ChangeLog entries are as follows: >> >> *** gcc/ChangeLog *** >> >> 2018-12-14 Thomas Preud'homme >> >> Revert: >> 2016-11-22 Thomas Preud'homme >> >> PR target/77904 >> * config/arm/arm.c (thumb1_compute_save_reg_mask): Mark frame pointer >> in save register mask if it is needed. >> >> *** gcc/testsuite/ChangeLog *** >> >> 2018-12-14 Thomas Preud'homme >> >> Revert: >> 2016-11-22 Thomas Preud'homme >> >> PR target/77904 >> * gcc.target/arm/pr77904.c: New test. >> >> Testing: Built an arm-none-eabi GCC cross-compiler targeting Armv6S-M >> and regression testsuite does not show any regression. >> >> Ok for stage3? > > OK. > > R. Bernd suggested in [1] that the behaviour tested by pr77904.c might actually be a behaviour we can allow with a patch to add a dg-warning to the decade. I'll wait for a resolution on that suggestion before deciding whether to commit this. Best regards, Thomas > >> >> Best regards, >> >> Thomas >> >> >> fix_pr77904_test_failure.patch >> >> From 63c52e7bf932947be7122cdc63f6cdc913479259 Mon Sep 17 00:00:00 2001 >> From: Thomas Preud'homme >> Date: Fri, 14 Dec 2018 16:02:59 +0000 >> Subject: [PATCH] [PATCH, ARM] Fix PR77904 testcase failure >> >> Hi, >> >> Commit r242693 forced fp to be saved/restored when needed due to an >> instance of GCC using fp as a scratch register to save sp while it's >> being clobbered by an inline asm. The normal path in >> thumb1_compute_save_reg_mask saving callee-saved registers which are >> live in the function does not work in that case because fp is chosen to >> hold sp after that function is called. >> >> Since clobbering sp is now errored out by the compiler and this was the >> only case reported where fp was live but not marked as such when >> thumb1_compute_save_reg_mask is called, I believe the whole commit >> r242693 should be reverted. >> >> ChangeLog entries are as follows: >> >> *** gcc/ChangeLog *** >> >> 2018-12-14 Thomas Preud'homme >> >> Revert: >> 2016-11-22 Thomas Preud'homme >> >> PR target/77904 >> * config/arm/arm.c (thumb1_compute_save_reg_mask): Mark frame pointer >> in save register mask if it is needed. >> >> *** gcc/testsuite/ChangeLog *** >> >> 2018-12-14 Thomas Preud'homme >> >> Revert: >> 2016-11-22 Thomas Preud'homme >> >> PR target/77904 >> * gcc.target/arm/pr77904.c: New test. >> >> Testing: Built an arm-none-eabi GCC cross-compiler targeting Armv6S-M >> and regression testsuite does not show any regression. >> >> Ok for stage3? >> >> Best regards, >> >> Thomas >> --- >> gcc/ChangeLog | 9 ++++++ >> gcc/config/arm/arm.c | 4 --- >> gcc/testsuite/ChangeLog | 8 +++++ >> gcc/testsuite/gcc.target/arm/pr77904.c | 45 -------------------------- >> 4 files changed, 17 insertions(+), 49 deletions(-) >> delete mode 100644 gcc/testsuite/gcc.target/arm/pr77904.c >> >> diff --git a/gcc/ChangeLog b/gcc/ChangeLog >> index d8e374fb15f..9caeb1d5e18 100644 >> --- a/gcc/ChangeLog >> +++ b/gcc/ChangeLog >> @@ -1,3 +1,12 @@ >> +2018-12-14 Thomas Preud'homme >> + >> + Revert: >> + 2016-11-22 Thomas Preud'homme >> + >> + PR target/77904 >> + * config/arm/arm.c (thumb1_compute_save_reg_mask): Mark frame pointer >> + in save register mask if it is needed. >> + >> 2018-11-27 Alan Modra >> >> * config/rs6000/aix71.h (ASM_SPEC): Don't select default -maix64 >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index 40f0574e32e..2ab5d8abc33 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -19553,10 +19553,6 @@ thumb1_compute_save_core_reg_mask (void) >> if (df_regs_ever_live_p (reg) && callee_saved_reg_p (reg)) >> mask |= 1 << reg; >> >> - /* Handle the frame pointer as a special case. */ >> - if (frame_pointer_needed) >> - mask |= 1 << HARD_FRAME_POINTER_REGNUM; >> - >> if (flag_pic >> && !TARGET_SINGLE_PIC_BASE >> && arm_pic_register != INVALID_REGNUM >> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog >> index 9e1f6d05a45..4e58c8940da 100644 >> --- a/gcc/testsuite/ChangeLog >> +++ b/gcc/testsuite/ChangeLog >> @@ -1,3 +1,11 @@ >> +2018-12-14 Thomas Preud'homme >> + >> + Revert: >> + 2016-11-22 Thomas Preud'homme >> + >> + PR target/77904 >> + * gcc.target/arm/pr77904.c: New test. >> + >> 2018-11-27 Jozef Lawrynowicz >> >> * lib/target-supports.exp >> diff --git a/gcc/testsuite/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c >> deleted file mode 100644 >> index 76728c07e73..00000000000 >> --- a/gcc/testsuite/gcc.target/arm/pr77904.c >> +++ /dev/null >> @@ -1,45 +0,0 @@ >> -/* { dg-do run } */ >> -/* { dg-options "-O2" } */ >> - >> -__attribute__ ((noinline, noclone)) void >> -clobber_sp (void) >> -{ >> - __asm volatile ("" : : : "sp"); >> -} >> - >> -int >> -main (void) >> -{ >> - int ret; >> - >> - __asm volatile ("mov\tr4, #0xf4\n\t" >> - "mov\tr5, #0xf5\n\t" >> - "mov\tr6, #0xf6\n\t" >> - "mov\tr7, #0xf7\n\t" >> - "mov\tr0, #0xf8\n\t" >> - "mov\tr8, r0\n\t" >> - "mov\tr0, #0xfa\n\t" >> - "mov\tr10, r0" >> - : : : "r0", "r4", "r5", "r6", "r7", "r8", "r10"); >> - clobber_sp (); >> - >> - __asm volatile ("cmp\tr4, #0xf4\n\t" >> - "bne\tfail\n\t" >> - "cmp\tr5, #0xf5\n\t" >> - "bne\tfail\n\t" >> - "cmp\tr6, #0xf6\n\t" >> - "bne\tfail\n\t" >> - "cmp\tr7, #0xf7\n\t" >> - "bne\tfail\n\t" >> - "mov\tr0, r8\n\t" >> - "cmp\tr0, #0xf8\n\t" >> - "bne\tfail\n\t" >> - "mov\tr0, r10\n\t" >> - "cmp\tr0, #0xfa\n\t" >> - "bne\tfail\n\t" >> - "mov\t%0, #1\n" >> - "fail:\n\t" >> - "sub\tr0, #1" >> - : "=r" (ret) : :); >> - return ret; >> -} >> > >