Hi Richard, On Thursday, 20 December 2018, Richard Earnshaw (lists) < richard.earns...@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 <thomas.preudho...@arm.com> >> >> Revert: >> 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> >> 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 <thomas.preudho...@arm.com> >> >> Revert: >> 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> >> 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 <thomas.preudho...@linaro.org> >> 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 <thomas.preudho...@arm.com> >> >> Revert: >> 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> >> 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 <thomas.preudho...@arm.com> >> >> Revert: >> 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> >> 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 <thomas.preudho...@arm.com> >> + >> + Revert: >> + 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> + >> + 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 <amo...@gmail.com> >> >> * 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 <thomas.preudho...@arm.com> >> + >> + Revert: >> + 2016-11-22 Thomas Preud'homme <thomas.preudho...@arm.com> >> + >> + PR target/77904 >> + * gcc.target/arm/pr77904.c: New test. >> + >> 2018-11-27 Jozef Lawrynowicz <joze...@mittosystems.com> >> >> * 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; >> -} >> > >