public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, ARM] Fix PR77904 testcase failure
@ 2018-12-14 23:28 Thomas Preudhomme
  2018-12-20 10:35 ` Richard Earnshaw (lists)
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Preudhomme @ 2018-12-14 23:28 UTC (permalink / raw)
  To: kyrylo.tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

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.preudhomme@arm.com>

    Revert:
    2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>

    Revert:
    2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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

[-- Attachment #2: fix_pr77904_test_failure.patch --]
[-- Type: text/x-patch, Size: 4579 bytes --]

From 63c52e7bf932947be7122cdc63f6cdc913479259 Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme@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.preudhomme@arm.com>

    Revert:
    2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>

    Revert:
    2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
+
+	Revert:
+	2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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  <amodra@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.preudhomme@arm.com>
+
+	Revert:
+	2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
+
+	PR target/77904
+	* gcc.target/arm/pr77904.c: New test.
+
 2018-11-27  Jozef Lawrynowicz  <jozef.l@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;
-}
-- 
2.19.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM] Fix PR77904 testcase failure
  2018-12-14 23:28 [PATCH, ARM] Fix PR77904 testcase failure Thomas Preudhomme
@ 2018-12-20 10:35 ` Richard Earnshaw (lists)
  2018-12-31 19:14   ` Thomas Preudhomme
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Earnshaw (lists) @ 2018-12-20 10:35 UTC (permalink / raw)
  To: Thomas Preudhomme, kyrylo.tkachov, Ramana Radhakrishnan, gcc-patches

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.preudhomme@arm.com>
> 
>     Revert:
>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
> 
>     Revert:
>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.

> 
> Best regards,
> 
> Thomas
> 
> 
> fix_pr77904_test_failure.patch
> 
> From 63c52e7bf932947be7122cdc63f6cdc913479259 Mon Sep 17 00:00:00 2001
> From: Thomas Preud'homme <thomas.preudhomme@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.preudhomme@arm.com>
> 
>     Revert:
>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
> 
>     Revert:
>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
> +
> +	Revert:
> +	2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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  <amodra@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.preudhomme@arm.com>
> +
> +	Revert:
> +	2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> +
> +	PR target/77904
> +	* gcc.target/arm/pr77904.c: New test.
> +
>  2018-11-27  Jozef Lawrynowicz  <jozef.l@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;
> -}
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM] Fix PR77904 testcase failure
  2018-12-20 10:35 ` Richard Earnshaw (lists)
@ 2018-12-31 19:14   ` Thomas Preudhomme
  2018-12-31 21:58     ` Thomas Preudhomme
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Preudhomme @ 2018-12-31 19:14 UTC (permalink / raw)
  To: Richard Earnshaw (lists)
  Cc: kyrylo.tkachov, Ramana Radhakrishnan, gcc-patches

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  <thomas.preudhomme@arm.com>
>>
>>     Revert:
>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>>
>>     Revert:
>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@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.preudhomme@arm.com>
>>
>>     Revert:
>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>>
>>     Revert:
>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>> +
>> +     Revert:
>> +     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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  <amodra@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.preudhomme@arm.com>
>> +
>> +     Revert:
>> +     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>> +
>> +     PR target/77904
>> +     * gcc.target/arm/pr77904.c: New test.
>> +
>>  2018-11-27  Jozef Lawrynowicz  <jozef.l@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;
>> -}
>>
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, ARM] Fix PR77904 testcase failure
  2018-12-31 19:14   ` Thomas Preudhomme
@ 2018-12-31 21:58     ` Thomas Preudhomme
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Preudhomme @ 2018-12-31 21:58 UTC (permalink / raw)
  To: Richard Earnshaw (lists)
  Cc: kyrylo.tkachov, Ramana Radhakrishnan, gcc-patches

Forgot the reference:

[1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01308.html

On Monday, 31 December 2018, Thomas Preudhomme <thomas.preudhomme@linaro.org>
wrote:
> 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  <thomas.preudhomme@arm.com>
>>>
>>>     Revert:
>>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>>>
>>>     Revert:
>>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@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.preudhomme@arm.com>
>>>
>>>     Revert:
>>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>>>
>>>     Revert:
>>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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.preudhomme@arm.com>
>>> +
>>> +     Revert:
>>> +     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@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  <amodra@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.preudhomme@arm.com>
>>> +
>>> +     Revert:
>>> +     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>> +
>>> +     PR target/77904
>>> +     * gcc.target/arm/pr77904.c: New test.
>>> +
>>>  2018-11-27  Jozef Lawrynowicz  <jozef.l@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;
>>> -}
>>>
>>
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-12-31 17:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14 23:28 [PATCH, ARM] Fix PR77904 testcase failure Thomas Preudhomme
2018-12-20 10:35 ` Richard Earnshaw (lists)
2018-12-31 19:14   ` Thomas Preudhomme
2018-12-31 21:58     ` Thomas Preudhomme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).