public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
@ 2016-11-03 16:52 Thomas Preudhomme
  2016-11-08 13:37 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
  2016-11-16 10:30 ` [PATCH, GCC/ARM] " Kyrill Tkachov
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-03 16:52 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Hi,

When using a callee-saved register to save the frame pointer the Thumb-1 
prologue fails to save the callee-saved register before that. For ARM and 
Thumb-2 targets the frame pointer is handled as a special case but nothing is 
done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.

ChangeLog entries are as follow:

*** gcc/ChangeLog ***

2016-11-02  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 ***

2016-11-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>

         PR target/77904
         * gcc.target/arm/pr77904.c: New test.


Testing: Testsuite shows no regression when run with arm-none-eabi GCC 
cross-compiler for Cortex-M0 target.

Is this ok for trunk?

Best regards,

Thomas

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

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dd8d5e5db8ca50daab648e58df290969aa794862..c7bf3320a3db5dfc4f33ae145ff2e5f239d6c0f9 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19495,6 +19495,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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] 12+ messages in thread

* Re: [PATCH, GCC/ARM, ping] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-03 16:52 [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp Thomas Preudhomme
@ 2016-11-08 13:37 ` Thomas Preudhomme
  2016-11-22 11:52   ` [arm-embedded] " Thomas Preudhomme
  2016-11-16 10:30 ` [PATCH, GCC/ARM] " Kyrill Tkachov
  1 sibling, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-08 13:37 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Ping?

Best regards,

Thomas

On 03/11/16 16:52, Thomas Preudhomme wrote:
> Hi,
>
> When using a callee-saved register to save the frame pointer the Thumb-1
> prologue fails to save the callee-saved register before that. For ARM and
> Thumb-2 targets the frame pointer is handled as a special case but nothing is
> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2016-11-02  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 ***
>
> 2016-11-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR target/77904
>         * gcc.target/arm/pr77904.c: New test.
>
>
> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
> cross-compiler for Cortex-M0 target.
>
> Is this ok for trunk?
>
> Best regards,
>
> Thomas

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

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dd8d5e5db8ca50daab648e58df290969aa794862..c7bf3320a3db5dfc4f33ae145ff2e5f239d6c0f9 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19495,6 +19495,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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] 12+ messages in thread

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-03 16:52 [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp Thomas Preudhomme
  2016-11-08 13:37 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
@ 2016-11-16 10:30 ` Kyrill Tkachov
  2016-11-17  8:56   ` Thomas Preudhomme
  1 sibling, 1 reply; 12+ messages in thread
From: Kyrill Tkachov @ 2016-11-16 10:30 UTC (permalink / raw)
  To: Thomas Preudhomme, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

Hi Thomas,

On 03/11/16 16:52, Thomas Preudhomme wrote:
> Hi,
>
> When using a callee-saved register to save the frame pointer the Thumb-1 prologue fails to save the callee-saved register before that. For ARM and Thumb-2 targets the frame pointer is handled as a special case but nothing is done for 
> Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2016-11-02  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.
>

s/mark/Mark/

>
> *** gcc/testsuite/ChangeLog ***
>
> 2016-11-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR target/77904
>         * gcc.target/arm/pr77904.c: New test.
>
>
> Testing: Testsuite shows no regression when run with arm-none-eabi GCC cross-compiler for Cortex-M0 target.
>
> Is this ok for trunk?
>

I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect anything.
Can you just double-check that the new test passes on non-Thumb-1 configurations as well?

Ok for trunk.
Thanks,
Kyrill

> Best regards,
>
> Thomas

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

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-16 10:30 ` [PATCH, GCC/ARM] " Kyrill Tkachov
@ 2016-11-17  8:56   ` Thomas Preudhomme
  2016-11-17  9:11     ` Kyrill Tkachov
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-17  8:56 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

On 16/11/16 10:30, Kyrill Tkachov wrote:
> Hi Thomas,
>
> On 03/11/16 16:52, Thomas Preudhomme wrote:
>> Hi,
>>
>> When using a callee-saved register to save the frame pointer the Thumb-1
>> prologue fails to save the callee-saved register before that. For ARM and
>> Thumb-2 targets the frame pointer is handled as a special case but nothing is
>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>>
>> ChangeLog entries are as follow:
>>
>> *** gcc/ChangeLog ***
>>
>> 2016-11-02  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.
>>
>
> s/mark/Mark/
>
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2016-11-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         PR target/77904
>>         * gcc.target/arm/pr77904.c: New test.
>>
>>
>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>> cross-compiler for Cortex-M0 target.
>>
>> Is this ok for trunk?
>>
>
> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
> anything.

I can bootstrap for armv4t with --with-mode=thumb which would at least exercise 
the path. I'll try such a bootstrap on qemu.

> Can you just double-check that the new test passes on non-Thumb-1 configurations
> as well?

It works well for ARMv7-A in ARM and Thumb mode.

Best regards,

Thomas

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

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-17  8:56   ` Thomas Preudhomme
@ 2016-11-17  9:11     ` Kyrill Tkachov
  2016-11-22 10:45       ` Thomas Preudhomme
  0 siblings, 1 reply; 12+ messages in thread
From: Kyrill Tkachov @ 2016-11-17  9:11 UTC (permalink / raw)
  To: Thomas Preudhomme, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches


On 17/11/16 08:56, Thomas Preudhomme wrote:
> On 16/11/16 10:30, Kyrill Tkachov wrote:
>> Hi Thomas,
>>
>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>> prologue fails to save the callee-saved register before that. For ARM and
>>> Thumb-2 targets the frame pointer is handled as a special case but nothing is
>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2016-11-02  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.
>>>
>>
>> s/mark/Mark/
>>
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>>         PR target/77904
>>>         * gcc.target/arm/pr77904.c: New test.
>>>
>>>
>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>> cross-compiler for Cortex-M0 target.
>>>
>>> Is this ok for trunk?
>>>
>>
>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>> anything.
>
> I can bootstrap for armv4t with --with-mode=thumb which would at least exercise the path. I'll try such a bootstrap on qemu.
>

If you can get it to work, then yes please.

>> Can you just double-check that the new test passes on non-Thumb-1 configurations
>> as well?
>
> It works well for ARMv7-A in ARM and Thumb mode.
>

Great, thanks again.
Kyrill

> Best regards,
>
> Thomas

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

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-17  9:11     ` Kyrill Tkachov
@ 2016-11-22 10:45       ` Thomas Preudhomme
  2016-11-30 10:42         ` Thomas Preudhomme
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-22 10:45 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

On 17/11/16 09:11, Kyrill Tkachov wrote:
>
> On 17/11/16 08:56, Thomas Preudhomme wrote:
>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>> Hi Thomas,
>>>
>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>> Hi,
>>>>
>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>> Thumb-2 targets the frame pointer is handled as a special case but nothing is
>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>>>>
>>>> ChangeLog entries are as follow:
>>>>
>>>> *** gcc/ChangeLog ***
>>>>
>>>> 2016-11-02  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.
>>>>
>>>
>>> s/mark/Mark/
>>>
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>>
>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>         PR target/77904
>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>
>>>>
>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>> cross-compiler for Cortex-M0 target.
>>>>
>>>> Is this ok for trunk?
>>>>
>>>
>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>> anything.
>>
>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>> exercise the path. I'll try such a bootstrap on qemu.
>>
>
> If you can get it to work, then yes please.

Bootstrap came back clean so I've committed the patch (r242693). Thanks!

Best regards,

Thomas

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

* [arm-embedded] [PATCH, GCC/ARM, ping] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-08 13:37 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
@ 2016-11-22 11:52   ` Thomas Preudhomme
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-22 11:52 UTC (permalink / raw)
  To: gcc-patches

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

Hi,

We have decided to backport this patch to fix callee-saved register corruption 
when clobbering sp to our embedded-6-branch.

*** gcc/ChangeLog.arm ***

         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.arm ***

         PR target/77904
         * gcc.target/arm/pr77904.c: New test.


Best regards,

Thomas

[-- Attachment #2: Re: [PATCH, GCC/ARM, ping] Fix PR77904: callee-saved register trashed when clobbering sp.eml --]
[-- Type: message/rfc822, Size: 4044 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1026 bytes --]

Ping?

Best regards,

Thomas

On 03/11/16 16:52, Thomas Preudhomme wrote:
> Hi,
>
> When using a callee-saved register to save the frame pointer the Thumb-1
> prologue fails to save the callee-saved register before that. For ARM and
> Thumb-2 targets the frame pointer is handled as a special case but nothing is
> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2016-11-02  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 ***
>
> 2016-11-02  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         PR target/77904
>         * gcc.target/arm/pr77904.c: New test.
>
>
> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
> cross-compiler for Cortex-M0 target.
>
> Is this ok for trunk?
>
> Best regards,
>
> Thomas

[-- Attachment #2.1.2: fix_pr77904.patch --]
[-- Type: text/x-patch, Size: 1783 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dd8d5e5db8ca50daab648e58df290969aa794862..c7bf3320a3db5dfc4f33ae145ff2e5f239d6c0f9 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19495,6 +19495,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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] 12+ messages in thread

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-22 10:45       ` Thomas Preudhomme
@ 2016-11-30 10:42         ` Thomas Preudhomme
  2016-11-30 10:44           ` Thomas Preudhomme
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-30 10:42 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

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

Hi,

Is this ok to backport to gcc-5-branch and gcc-6-branch? Patch applies cleanly 
(patches attached for reference).


2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com>

     Backport from mainline
     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>

     gcc/
     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/
     PR target/77904
     * gcc.target/arm/pr77904.c: New test.


Best regards,

Thomas


On 22/11/16 10:45, Thomas Preudhomme wrote:
> On 17/11/16 09:11, Kyrill Tkachov wrote:
>>
>> On 17/11/16 08:56, Thomas Preudhomme wrote:
>>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>>> Hi Thomas,
>>>>
>>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>>> Hi,
>>>>>
>>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>>> Thumb-2 targets the frame pointer is handled as a special case but nothing is
>>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>>>>>
>>>>> ChangeLog entries are as follow:
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2016-11-02  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.
>>>>>
>>>>
>>>> s/mark/Mark/
>>>>
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>>         PR target/77904
>>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>>
>>>>>
>>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>>> cross-compiler for Cortex-M0 target.
>>>>>
>>>>> Is this ok for trunk?
>>>>>
>>>>
>>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>>> anything.
>>>
>>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>>> exercise the path. I'll try such a bootstrap on qemu.
>>>
>>
>> If you can get it to work, then yes please.
>
> Bootstrap came back clean so I've committed the patch (r242693). Thanks!
>
> Best regards,
>
> Thomas

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

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c01a3c878968f6e6f07358b0686e4a59e34f56b7..62fd5c557e4a356bc2bf0141d35b959dfb859842 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19361,6 +19361,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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;
+}

[-- Attachment #3: fix_pr77904_gcc6.patch --]
[-- Type: text/x-patch, Size: 1783 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 83cb13d1195beb19d6301f5c83a7eb544a91d877..ae479a43fe8514f2eacb7d56f89916b48f720768 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19390,6 +19390,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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] 12+ messages in thread

* Re: [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-30 10:42         ` Thomas Preudhomme
@ 2016-11-30 10:44           ` Thomas Preudhomme
  2016-12-06 11:38             ` [PATCH, GCC/ARM, gcc-5/6-branch, ping] " Thomas Preudhomme
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-11-30 10:44 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

Sorry, the bug cannot be reproduced on gcc-5-branch so it's probably better to 
only do a backport to gcc-6-branch.

Ok for a backport to gcc-6-branch?

Best regards,

Thomas

On 30/11/16 10:42, Thomas Preudhomme wrote:
> Hi,
>
> Is this ok to backport to gcc-5-branch and gcc-6-branch? Patch applies cleanly
> (patches attached for reference).
>
>
> 2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
>     Backport from mainline
>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>     gcc/
>     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/
>     PR target/77904
>     * gcc.target/arm/pr77904.c: New test.
>
>
> Best regards,
>
> Thomas
>
>
> On 22/11/16 10:45, Thomas Preudhomme wrote:
>> On 17/11/16 09:11, Kyrill Tkachov wrote:
>>>
>>> On 17/11/16 08:56, Thomas Preudhomme wrote:
>>>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>>>> Hi,
>>>>>>
>>>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>>>> Thumb-2 targets the frame pointer is handled as a special case but nothing is
>>>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1 targets.
>>>>>>
>>>>>> ChangeLog entries are as follow:
>>>>>>
>>>>>> *** gcc/ChangeLog ***
>>>>>>
>>>>>> 2016-11-02  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.
>>>>>>
>>>>>
>>>>> s/mark/Mark/
>>>>>
>>>>>>
>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>
>>>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>
>>>>>>         PR target/77904
>>>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>>>
>>>>>>
>>>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>>>> cross-compiler for Cortex-M0 target.
>>>>>>
>>>>>> Is this ok for trunk?
>>>>>>
>>>>>
>>>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>>>> anything.
>>>>
>>>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>>>> exercise the path. I'll try such a bootstrap on qemu.
>>>>
>>>
>>> If you can get it to work, then yes please.
>>
>> Bootstrap came back clean so I've committed the patch (r242693). Thanks!
>>
>> Best regards,
>>
>> Thomas

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

* Re: [PATCH, GCC/ARM, gcc-5/6-branch, ping] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-11-30 10:44           ` Thomas Preudhomme
@ 2016-12-06 11:38             ` Thomas Preudhomme
  2016-12-12 10:44               ` Thomas Preudhomme
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-12-06 11:38 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw; +Cc: gcc-patches

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

Ping?

Best regards,

Thomas

On 30/11/16 10:44, Thomas Preudhomme wrote:
> Sorry, the bug cannot be reproduced on gcc-5-branch so it's probably better to
> only do a backport to gcc-6-branch.
>
> Ok for a backport to gcc-6-branch?
>
> Best regards,
>
> Thomas
>
> On 30/11/16 10:42, Thomas Preudhomme wrote:
>> Hi,
>>
>> Is this ok to backport to gcc-5-branch and gcc-6-branch? Patch applies cleanly
>> (patches attached for reference).
>>
>>
>> 2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>
>>     Backport from mainline
>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>     gcc/
>>     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/
>>     PR target/77904
>>     * gcc.target/arm/pr77904.c: New test.
>>
>>
>> Best regards,
>>
>> Thomas
>>
>>
>> On 22/11/16 10:45, Thomas Preudhomme wrote:
>>> On 17/11/16 09:11, Kyrill Tkachov wrote:
>>>>
>>>> On 17/11/16 08:56, Thomas Preudhomme wrote:
>>>>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>>>>> Hi Thomas,
>>>>>>
>>>>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>>>>> Thumb-2 targets the frame pointer is handled as a special case but
>>>>>>> nothing is
>>>>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1
>>>>>>> targets.
>>>>>>>
>>>>>>> ChangeLog entries are as follow:
>>>>>>>
>>>>>>> *** gcc/ChangeLog ***
>>>>>>>
>>>>>>> 2016-11-02  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.
>>>>>>>
>>>>>>
>>>>>> s/mark/Mark/
>>>>>>
>>>>>>>
>>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>>
>>>>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>
>>>>>>>         PR target/77904
>>>>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>>>>
>>>>>>>
>>>>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>>>>> cross-compiler for Cortex-M0 target.
>>>>>>>
>>>>>>> Is this ok for trunk?
>>>>>>>
>>>>>>
>>>>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>>>>> anything.
>>>>>
>>>>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>>>>> exercise the path. I'll try such a bootstrap on qemu.
>>>>>
>>>>
>>>> If you can get it to work, then yes please.
>>>
>>> Bootstrap came back clean so I've committed the patch (r242693). Thanks!
>>>
>>> Best regards,
>>>
>>> Thomas

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

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 47171b99682207226aa4f9a76d4dfb54d6c2814b..86df1c0366be6c4b9b4ebf76821a8100c4e9fc16 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -575,9 +575,9 @@
 ;;  (plus (reg rN) (reg sp)) into (reg rN).  In this case reload will
 ;; put the duplicated register first, and not try the commutative version.
 (define_insn_and_split "*arm_addsi3"
-  [(set (match_operand:SI          0 "s_register_operand" "=rk,l,l ,l ,r ,k ,r,r ,k ,r ,k,k,r ,k ,r")
-        (plus:SI (match_operand:SI 1 "s_register_operand" "%0 ,l,0 ,l ,rk,k ,r,rk,k ,rk,k,r,rk,k ,rk")
-                 (match_operand:SI 2 "reg_or_int_operand" "rk ,l,Py,Pd,rI,rI,k,Pj,Pj,L ,L,L,PJ,PJ,?n")))]
+  [(set (match_operand:SI          0 "s_register_operand" "=rk,l,l ,l ,r ,k ,r,k ,r ,k ,r ,k,k,r ,k ,r")
+	(plus:SI (match_operand:SI 1 "s_register_operand" "%0 ,l,0 ,l ,rk,k ,r,r ,rk,k ,rk,k,r,rk,k ,rk")
+		 (match_operand:SI 2 "reg_or_int_operand" "rk ,l,Py,Pd,rI,rI,k,rI,Pj,Pj,L ,L,L,PJ,PJ,?n")))]
   "TARGET_32BIT"
   "@
    add%?\\t%0, %0, %2
@@ -587,6 +587,7 @@
    add%?\\t%0, %1, %2
    add%?\\t%0, %1, %2
    add%?\\t%0, %2, %1
+   add%?\\t%0, %1, %2
    addw%?\\t%0, %1, %2
    addw%?\\t%0, %1, %2
    sub%?\\t%0, %1, #%n2
@@ -606,10 +607,10 @@
 		      operands[1], 0);
   DONE;
   "
-  [(set_attr "length" "2,4,4,4,4,4,4,4,4,4,4,4,4,4,16")
+  [(set_attr "length" "2,4,4,4,4,4,4,4,4,4,4,4,4,4,4,16")
    (set_attr "predicable" "yes")
-   (set_attr "predicable_short_it" "yes,yes,yes,yes,no,no,no,no,no,no,no,no,no,no,no")
-   (set_attr "arch" "t2,t2,t2,t2,*,*,*,t2,t2,*,*,a,t2,t2,*")
+   (set_attr "predicable_short_it" "yes,yes,yes,yes,no,no,no,no,no,no,no,no,no,no,no,no")
+   (set_attr "arch" "t2,t2,t2,t2,*,*,*,a,t2,t2,*,*,a,t2,t2,*")
    (set (attr "type") (if_then_else (match_operand 2 "const_int_operand" "")
 		      (const_string "alu_imm")
 		      (const_string "alu_sreg")))
diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
new file mode 100644
index 0000000000000000000000000000000000000000..8313f2199122be153a737946e817a5e3bee60372
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { ! arm_cortex_m } { "-mthumb" } } */
+
+/* Below code used to trigger an ICE due to missing constraints for
+   sp = fp + cst pattern.  */
+
+void fiq_handler (void) __attribute__((interrupt ("FIQ")));
+
+void
+fiq_handler (void)
+{
+}

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

* Re: [PATCH, GCC/ARM, gcc-5/6-branch, ping] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-12-06 11:38             ` [PATCH, GCC/ARM, gcc-5/6-branch, ping] " Thomas Preudhomme
@ 2016-12-12 10:44               ` Thomas Preudhomme
  2016-12-12 10:46                 ` Kyrill Tkachov
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Preudhomme @ 2016-12-12 10:44 UTC (permalink / raw)
  To: Kyrill Tkachov, Ramana Radhakrishnan, Richard Earnshaw; +Cc: gcc-patches

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

Same as for PR77933, I'm trying to get this in for the next GCC 6 release.

I've successfully done a Thumb-1 bootstrap of the backport and testsuite shows 
no regression when run with an arm-none-eabi GCC cross-compiler targeting ARM 
Cortex-M0.

Is this ok for gcc-6-branch?

Best regards,

Thomas

On 06/12/16 11:38, Thomas Preudhomme wrote:
> Ping?
>
> Best regards,
>
> Thomas
>
> On 30/11/16 10:44, Thomas Preudhomme wrote:
>> Sorry, the bug cannot be reproduced on gcc-5-branch so it's probably better to
>> only do a backport to gcc-6-branch.
>>
>> Ok for a backport to gcc-6-branch?
>>
>> Best regards,
>>
>> Thomas
>>
>> On 30/11/16 10:42, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> Is this ok to backport to gcc-5-branch and gcc-6-branch? Patch applies cleanly
>>> (patches attached for reference).
>>>
>>>
>>> 2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>>     Backport from mainline
>>>     2016-11-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>     gcc/
>>>     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/
>>>     PR target/77904
>>>     * gcc.target/arm/pr77904.c: New test.
>>>
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>>
>>> On 22/11/16 10:45, Thomas Preudhomme wrote:
>>>> On 17/11/16 09:11, Kyrill Tkachov wrote:
>>>>>
>>>>> On 17/11/16 08:56, Thomas Preudhomme wrote:
>>>>>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>>>>>> Hi Thomas,
>>>>>>>
>>>>>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>>>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>>>>>> Thumb-2 targets the frame pointer is handled as a special case but
>>>>>>>> nothing is
>>>>>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1
>>>>>>>> targets.
>>>>>>>>
>>>>>>>> ChangeLog entries are as follow:
>>>>>>>>
>>>>>>>> *** gcc/ChangeLog ***
>>>>>>>>
>>>>>>>> 2016-11-02  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.
>>>>>>>>
>>>>>>>
>>>>>>> s/mark/Mark/
>>>>>>>
>>>>>>>>
>>>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>>>
>>>>>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>>
>>>>>>>>         PR target/77904
>>>>>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>>>>>
>>>>>>>>
>>>>>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>>>>>> cross-compiler for Cortex-M0 target.
>>>>>>>>
>>>>>>>> Is this ok for trunk?
>>>>>>>>
>>>>>>>
>>>>>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>>>>>> anything.
>>>>>>
>>>>>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>>>>>> exercise the path. I'll try such a bootstrap on qemu.
>>>>>>
>>>>>
>>>>> If you can get it to work, then yes please.
>>>>
>>>> Bootstrap came back clean so I've committed the patch (r242693). Thanks!
>>>>
>>>> Best regards,
>>>>
>>>> Thomas

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

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 83cb13d1195beb19d6301f5c83a7eb544a91d877..ae479a43fe8514f2eacb7d56f89916b48f720768 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -19390,6 +19390,10 @@ thumb1_compute_save_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/gcc.target/arm/pr77904.c b/gcc/testsuite/gcc.target/arm/pr77904.c
new file mode 100644
index 0000000000000000000000000000000000000000..76728c07e73350ce44160cabff3dd2fa7a6ef021
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr77904.c
@@ -0,0 +1,45 @@
+/* { 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] 12+ messages in thread

* Re: [PATCH, GCC/ARM, gcc-5/6-branch, ping] Fix PR77904: callee-saved register trashed when clobbering sp
  2016-12-12 10:44               ` Thomas Preudhomme
@ 2016-12-12 10:46                 ` Kyrill Tkachov
  0 siblings, 0 replies; 12+ messages in thread
From: Kyrill Tkachov @ 2016-12-12 10:46 UTC (permalink / raw)
  To: Thomas Preudhomme, Ramana Radhakrishnan, Richard Earnshaw; +Cc: gcc-patches


On 12/12/16 10:44, Thomas Preudhomme wrote:
> Same as for PR77933, I'm trying to get this in for the next GCC 6 release.
>
> I've successfully done a Thumb-1 bootstrap of the backport and testsuite shows no regression when run with an arm-none-eabi GCC cross-compiler targeting ARM Cortex-M0.
>
> Is this ok for gcc-6-branch?
>

Given this has been in trunk for some time, is specific to Thumb-1 and fixes a wrong-code issue this is ok.
Thanks,
Kyrill

> Best regards,
>
> Thomas
>
> On 06/12/16 11:38, Thomas Preudhomme wrote:
>> Ping?
>>
>> Best regards,
>>
>> Thomas
>>
>> On 30/11/16 10:44, Thomas Preudhomme wrote:
>>> Sorry, the bug cannot be reproduced on gcc-5-branch so it's probably better to
>>> only do a backport to gcc-6-branch.
>>>
>>> Ok for a backport to gcc-6-branch?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 30/11/16 10:42, Thomas Preudhomme wrote:
>>>> Hi,
>>>>
>>>> Is this ok to backport to gcc-5-branch and gcc-6-branch? Patch applies cleanly
>>>> (patches attached for reference).
>>>>
>>>>
>>>> 2016-11-30 Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>     Backport from mainline
>>>>     2016-11-22  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>     gcc/
>>>>     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/
>>>>     PR target/77904
>>>>     * gcc.target/arm/pr77904.c: New test.
>>>>
>>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>>
>>>> On 22/11/16 10:45, Thomas Preudhomme wrote:
>>>>> On 17/11/16 09:11, Kyrill Tkachov wrote:
>>>>>>
>>>>>> On 17/11/16 08:56, Thomas Preudhomme wrote:
>>>>>>> On 16/11/16 10:30, Kyrill Tkachov wrote:
>>>>>>>> Hi Thomas,
>>>>>>>>
>>>>>>>> On 03/11/16 16:52, Thomas Preudhomme wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> When using a callee-saved register to save the frame pointer the Thumb-1
>>>>>>>>> prologue fails to save the callee-saved register before that. For ARM and
>>>>>>>>> Thumb-2 targets the frame pointer is handled as a special case but
>>>>>>>>> nothing is
>>>>>>>>> done for Thumb-1 targets. This patch adds the same logic for Thumb-1
>>>>>>>>> targets.
>>>>>>>>>
>>>>>>>>> ChangeLog entries are as follow:
>>>>>>>>>
>>>>>>>>> *** gcc/ChangeLog ***
>>>>>>>>>
>>>>>>>>> 2016-11-02  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.
>>>>>>>>>
>>>>>>>>
>>>>>>>> s/mark/Mark/
>>>>>>>>
>>>>>>>>>
>>>>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>>>>
>>>>>>>>> 2016-11-02  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>>>
>>>>>>>>>         PR target/77904
>>>>>>>>>         * gcc.target/arm/pr77904.c: New test.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Testing: Testsuite shows no regression when run with arm-none-eabi GCC
>>>>>>>>> cross-compiler for Cortex-M0 target.
>>>>>>>>>
>>>>>>>>> Is this ok for trunk?
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'd ask for a bootstrap, but this code is Thumb-1 only so it wouldn't affect
>>>>>>>> anything.
>>>>>>>
>>>>>>> I can bootstrap for armv4t with --with-mode=thumb which would at least
>>>>>>> exercise the path. I'll try such a bootstrap on qemu.
>>>>>>>
>>>>>>
>>>>>> If you can get it to work, then yes please.
>>>>>
>>>>> Bootstrap came back clean so I've committed the patch (r242693). Thanks!
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Thomas

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

end of thread, other threads:[~2016-12-12 10:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03 16:52 [PATCH, GCC/ARM] Fix PR77904: callee-saved register trashed when clobbering sp Thomas Preudhomme
2016-11-08 13:37 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2016-11-22 11:52   ` [arm-embedded] " Thomas Preudhomme
2016-11-16 10:30 ` [PATCH, GCC/ARM] " Kyrill Tkachov
2016-11-17  8:56   ` Thomas Preudhomme
2016-11-17  9:11     ` Kyrill Tkachov
2016-11-22 10:45       ` Thomas Preudhomme
2016-11-30 10:42         ` Thomas Preudhomme
2016-11-30 10:44           ` Thomas Preudhomme
2016-12-06 11:38             ` [PATCH, GCC/ARM, gcc-5/6-branch, ping] " Thomas Preudhomme
2016-12-12 10:44               ` Thomas Preudhomme
2016-12-12 10:46                 ` Kyrill Tkachov

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).