| Message ID | 000001d02fe3$c12d7d90$438878b0$@arm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme <thomas.preudhomme@arm.com> wrote: > When compiling for size, live high registers are not saved in function prolog in ARM backend in Thumb mode. The problem comes from arm_conditional_register_usage setting call_used_regs for all high register to avoid them being allocated. However, this cause prolog to not save these register even if they are used. This patch marks high registers as really needing to be saved in prolog if live, no matter what is the content of call_used_regs. > > ChangeLog entries are as follows: > > gcc/ChangeLog > > 2015-01-12 Thomas Preud'homme thomas.preudhomme@arm.com > > PR target/64453 > * config/arm/arm.c (callee_saved_reg_p): Define. > (arm_compute_save_reg0_reg12_mask): Use callee_saved_reg_p to check if > register is callee saved instead of !call_used_regs[reg]. > (thumb1_compute_save_reg_mask): Likewise. > > > gcc/testsuite/ChangeLog > > 2014-12-31 Thomas Preud'homme thomas.preudhomme@arm.com > > * gcc.target/arm/pr64453.c: New. > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 0ec526b..fcc14c2 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -18989,6 +18989,14 @@ output_ascii_pseudo_op (FILE *stream, const unsigned char *p, int len) > fputs ("\"\n", stream); > } > > > +/* Whether a register is callee saved or not. This is necessary because high > + registers are marked as caller saved when optimizing for size on Thumb-1 > + targets despite being callee saved in order to avoid using them. */ > +#define callee_saved_reg_p(reg) \ > + (!call_used_regs[reg] \ > + || (TARGET_THUMB1 && optimize_size \ > + && reg >= FIRST_HI_REGNUM && reg <= LAST_HI_REGNUM)) > + > /* Compute the register save mask for registers 0 through 12 > inclusive. This code is used by arm_compute_save_reg_mask. */ > > @@ -19049,7 +19057,7 @@ arm_compute_save_reg0_reg12_mask (void) > /* In the normal case we only need to save those registers > which are call saved and which are used by this function. */ > for (reg = 0; reg <= 11; reg++) > - if (df_regs_ever_live_p (reg) && ! call_used_regs[reg]) > + if (df_regs_ever_live_p (reg) && callee_saved_reg_p (reg)) > save_reg_mask |= (1 << reg); > > /* Handle the frame pointer as a special case. */ > @@ -19212,7 +19220,7 @@ thumb1_compute_save_reg_mask (void) > > mask = 0; > for (reg = 0; reg < 12; reg ++) > - if (df_regs_ever_live_p (reg) && !call_used_regs[reg]) > + if (df_regs_ever_live_p (reg) && callee_saved_reg_p (reg)) > mask |= 1 << reg; > > if (flag_pic > @@ -19245,7 +19253,7 @@ thumb1_compute_save_reg_mask (void) > if (reg * UNITS_PER_WORD <= (unsigned) arm_size_return_regs ()) > reg = LAST_LO_REGNUM; > > - if (! call_used_regs[reg]) > + if (callee_saved_reg_p (reg)) > mask |= 1 << reg; > } > > @@ -27185,8 +27193,7 @@ arm_conditional_register_usage (void) > /* When optimizing for size on Thumb-1, it's better not > to use the HI regs, because of the overhead of > stacking them. */ > - for (regno = FIRST_HI_REGNUM; > - regno <= LAST_HI_REGNUM; ++regno) > + for (regno = FIRST_HI_REGNUM; regno <= LAST_HI_REGNUM; ++regno) > fixed_regs[regno] = call_used_regs[regno] = 1; > } > > diff --git a/gcc/testsuite/gcc.target/arm/pr64453.c b/gcc/testsuite/gcc.target/arm/pr64453.c > new file mode 100644 > index 0000000..17155af > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/pr64453.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mthumb -Os " } */ > +/* { dg-require-effective-target arm_thumb1_ok } */ > + > +void save_regs () { > + __asm volatile ("" ::: "r8"); > +} > + > +/* { dg-final { scan-assembler "\tmov\tr., r8" } } */ > > Tested by compiling an arm-none-eabi GCC cross-compiler and running the testsuite on QEMU emulating Cortex-M0 without any regression. > > Is this ok for trunk? > > Best regards, > > Thomas > > > OK. Ramana
Hi Ramana, > From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > Sent: Wednesday, January 14, 2015 7:21 PM > On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > <thomas.preudhomme@arm.com> wrote: > > When compiling for size, live high registers are not saved in function > prolog in ARM backend in Thumb mode. The problem comes from > arm_conditional_register_usage setting call_used_regs for all high > register to avoid them being allocated. However, this cause prolog to not > save these register even if they are used. This patch marks high registers > as really needing to be saved in prolog if live, no matter what is the > content of call_used_regs. > > > > ChangeLog entries are as follows: > > > > gcc/ChangeLog > > > > 2015-01-12 Thomas Preud'homme thomas.preudhomme@arm.com > > > > PR target/64453 > > * config/arm/arm.c (callee_saved_reg_p): Define. > > (arm_compute_save_reg0_reg12_mask): Use callee_saved_reg_p > to check if > > register is callee saved instead of !call_used_regs[reg]. > > (thumb1_compute_save_reg_mask): Likewise. > > > > > > gcc/testsuite/ChangeLog > > > > 2014-12-31 Thomas Preud'homme thomas.preudhomme@arm.com > > > > * gcc.target/arm/pr64453.c: New. > > > > > > > > OK. > > Ramana The patch applies cleanly on GCC 4.8 and 4.9 branches when omitting the cosmetic change in arm_conditional_register_usage () which was unintended. I compiled an arm-none-eabi GCC cross compiler and ran the testsuite for both backport without any regression. Is this ok for the 4.8 and 4.9 branches? Best regards, Thomas
On Fri, Jan 23, 2015 at 8:23 AM, Thomas Preud'homme <thomas.preudhomme@arm.com> wrote: > Hi Ramana, > >> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] >> Sent: Wednesday, January 14, 2015 7:21 PM >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme >> <thomas.preudhomme@arm.com> wrote: >> > When compiling for size, live high registers are not saved in function >> prolog in ARM backend in Thumb mode. The problem comes from >> arm_conditional_register_usage setting call_used_regs for all high >> register to avoid them being allocated. However, this cause prolog to not >> save these register even if they are used. This patch marks high registers >> as really needing to be saved in prolog if live, no matter what is the >> content of call_used_regs. >> > >> > ChangeLog entries are as follows: >> > >> > gcc/ChangeLog >> > >> > 2015-01-12 Thomas Preud'homme thomas.preudhomme@arm.com >> > >> > PR target/64453 >> > * config/arm/arm.c (callee_saved_reg_p): Define. >> > (arm_compute_save_reg0_reg12_mask): Use callee_saved_reg_p >> to check if >> > register is callee saved instead of !call_used_regs[reg]. >> > (thumb1_compute_save_reg_mask): Likewise. >> > >> > >> > gcc/testsuite/ChangeLog >> > >> > 2014-12-31 Thomas Preud'homme thomas.preudhomme@arm.com >> > >> > * gcc.target/arm/pr64453.c: New. >> > >> > >> > >> >> OK. >> >> Ramana > > The patch applies cleanly on GCC 4.8 and 4.9 branches when omitting the cosmetic change > in arm_conditional_register_usage () which was unintended. I compiled an arm-none-eabi > GCC cross compiler and ran the testsuite for both backport without any regression. > > Is this ok for the 4.8 and 4.9 branches? > OK for the branches if no RM objects in 24 hours. Ramana > Best regards, > > Thomas > > >
Just committed to 4.9 branch, 4.8 to follow once regression testsuite for 4.8 backport finishes running (backport was done quite some time ago now). Best regards, Thomas > -----Original Message----- > From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > Sent: Tuesday, February 17, 2015 4:07 PM > To: Thomas Preud'homme > Cc: Ramana Radhakrishnan; gcc-patches; Richard Biener; Jakub Jelinek > Subject: Re: [PATCH, ARM] Fix PR64453: live high register not saved in > function prolog with -Os > > On Fri, Jan 23, 2015 at 8:23 AM, Thomas Preud'homme > <thomas.preudhomme@arm.com> wrote: > > Hi Ramana, > > > >> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > >> Sent: Wednesday, January 14, 2015 7:21 PM > >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > >> <thomas.preudhomme@arm.com> wrote: > >> > When compiling for size, live high registers are not saved in function > >> prolog in ARM backend in Thumb mode. The problem comes from > >> arm_conditional_register_usage setting call_used_regs for all high > >> register to avoid them being allocated. However, this cause prolog to > not > >> save these register even if they are used. This patch marks high > registers > >> as really needing to be saved in prolog if live, no matter what is the > >> content of call_used_regs. > >> > > >> > ChangeLog entries are as follows: > >> > > >> > gcc/ChangeLog > >> > > >> > 2015-01-12 Thomas Preud'homme thomas.preudhomme@arm.com > >> > > >> > PR target/64453 > >> > * config/arm/arm.c (callee_saved_reg_p): Define. > >> > (arm_compute_save_reg0_reg12_mask): Use > callee_saved_reg_p > >> to check if > >> > register is callee saved instead of !call_used_regs[reg]. > >> > (thumb1_compute_save_reg_mask): Likewise. > >> > > >> > > >> > gcc/testsuite/ChangeLog > >> > > >> > 2014-12-31 Thomas Preud'homme thomas.preudhomme@arm.com > >> > > >> > * gcc.target/arm/pr64453.c: New. > >> > > >> > > >> > > >> > >> OK. > >> > >> Ramana > > > > The patch applies cleanly on GCC 4.8 and 4.9 branches when omitting > the cosmetic change > > in arm_conditional_register_usage () which was unintended. I > compiled an arm-none-eabi > > GCC cross compiler and ran the testsuite for both backport without any > regression. > > > > Is this ok for the 4.8 and 4.9 branches? > > > > OK for the branches if no RM objects in 24 hours. > > Ramana > > > Best regards, > > > > Thomas > > > > > >
Done for backport to 4.8. Best regards, Thomas > -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme > Sent: Tuesday, March 03, 2015 5:35 PM > To: Ramana Radhakrishnan > Cc: gcc-patches; Richard Biener; Jakub Jelinek > Subject: RE: [PATCH, ARM] Fix PR64453: live high register not saved in > function prolog with -Os > > Just committed to 4.9 branch, 4.8 to follow once regression testsuite for > 4.8 backport finishes running (backport was done quite some time ago > now). > > Best regards, > > Thomas > > > -----Original Message----- > > From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > > Sent: Tuesday, February 17, 2015 4:07 PM > > To: Thomas Preud'homme > > Cc: Ramana Radhakrishnan; gcc-patches; Richard Biener; Jakub Jelinek > > Subject: Re: [PATCH, ARM] Fix PR64453: live high register not saved in > > function prolog with -Os > > > > On Fri, Jan 23, 2015 at 8:23 AM, Thomas Preud'homme > > <thomas.preudhomme@arm.com> wrote: > > > Hi Ramana, > > > > > >> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > > >> Sent: Wednesday, January 14, 2015 7:21 PM > > >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > > >> <thomas.preudhomme@arm.com> wrote: > > >> > When compiling for size, live high registers are not saved in > function > > >> prolog in ARM backend in Thumb mode. The problem comes from > > >> arm_conditional_register_usage setting call_used_regs for all high > > >> register to avoid them being allocated. However, this cause prolog > to > > not > > >> save these register even if they are used. This patch marks high > > registers > > >> as really needing to be saved in prolog if live, no matter what is the > > >> content of call_used_regs. > > >> > > > >> > ChangeLog entries are as follows: > > >> > > > >> > gcc/ChangeLog > > >> > > > >> > 2015-01-12 Thomas Preud'homme > thomas.preudhomme@arm.com > > >> > > > >> > PR target/64453 > > >> > * config/arm/arm.c (callee_saved_reg_p): Define. > > >> > (arm_compute_save_reg0_reg12_mask): Use > > callee_saved_reg_p > > >> to check if > > >> > register is callee saved instead of !call_used_regs[reg]. > > >> > (thumb1_compute_save_reg_mask): Likewise. > > >> > > > >> > > > >> > gcc/testsuite/ChangeLog > > >> > > > >> > 2014-12-31 Thomas Preud'homme > thomas.preudhomme@arm.com > > >> > > > >> > * gcc.target/arm/pr64453.c: New. > > >> > > > >> > > > >> > > > >> > > >> OK. > > >> > > >> Ramana > > > > > > The patch applies cleanly on GCC 4.8 and 4.9 branches when omitting > > the cosmetic change > > > in arm_conditional_register_usage () which was unintended. I > > compiled an arm-none-eabi > > > GCC cross compiler and ran the testsuite for both backport without > any > > regression. > > > > > > Is this ok for the 4.8 and 4.9 branches? > > > > > > > OK for the branches if no RM objects in 24 hours. > > > > Ramana > > > > > Best regards, > > > > > > Thomas > > > > > > > > > > > > >
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 0ec526b..fcc14c2 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -18989,6 +18989,14 @@ output_ascii_pseudo_op (FILE *stream, const unsigned char *p, int len) fputs ("\"\n", stream); } +/* Whether a register is callee saved or not. This is necessary because high + registers are marked as caller saved when optimizing for size on Thumb-1 + targets despite being callee saved in order to avoid using them. */ +#define callee_saved_reg_p(reg) \ + (!call_used_regs[reg] \ + || (TARGET_THUMB1 && optimize_size \ + && reg >= FIRST_HI_REGNUM && reg <= LAST_HI_REGNUM)) + /* Compute the register save mask for registers 0 through 12 inclusive. This code is used by arm_compute_save_reg_mask. */ @@ -19049,7 +19057,7 @@ arm_compute_save_reg0_reg12_mask (void) /* In the normal case we only need to save those registers which are call saved and which are used by this function. */ for (reg = 0; reg <= 11; reg++) - if (df_regs_ever_live_p (reg) && ! call_used_regs[reg]) + if (df_regs_ever_live_p (reg) && callee_saved_reg_p (reg)) save_reg_mask |= (1 << reg); /* Handle the frame pointer as a special case. */ @@ -19212,7 +19220,7 @@ thumb1_compute_save_reg_mask (void) mask = 0; for (reg = 0; reg < 12; reg ++) - if (df_regs_ever_live_p (reg) && !call_used_regs[reg]) + if (df_regs_ever_live_p (reg) && callee_saved_reg_p (reg)) mask |= 1 << reg; if (flag_pic @@ -19245,7 +19253,7 @@ thumb1_compute_save_reg_mask (void) if (reg * UNITS_PER_WORD <= (unsigned) arm_size_return_regs ()) reg = LAST_LO_REGNUM; - if (! call_used_regs[reg]) + if (callee_saved_reg_p (reg)) mask |= 1 << reg; } @@ -27185,8 +27193,7 @@ arm_conditional_register_usage (void) /* When optimizing for size on Thumb-1, it's better not to use the HI regs, because of the overhead of stacking them. */ - for (regno = FIRST_HI_REGNUM; - regno <= LAST_HI_REGNUM; ++regno) + for (regno = FIRST_HI_REGNUM; regno <= LAST_HI_REGNUM; ++regno) fixed_regs[regno] = call_used_regs[regno] = 1; } diff --git a/gcc/testsuite/gcc.target/arm/pr64453.c b/gcc/testsuite/gcc.target/arm/pr64453.c new file mode 100644 index 0000000..17155af --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr64453.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mthumb -Os " } */ +/* { dg-require-effective-target arm_thumb1_ok } */ + +void save_regs () { + __asm volatile ("" ::: "r8"); +} + +/* { dg-final { scan-assembler "\tmov\tr., r8" } } */
When compiling for size, live high registers are not saved in function prolog in ARM backend in Thumb mode. The problem comes from arm_conditional_register_usage setting call_used_regs for all high register to avoid them being allocated. However, this cause prolog to not save these register even if they are used. This patch marks high registers as really needing to be saved in prolog if live, no matter what is the content of call_used_regs. ChangeLog entries are as follows: gcc/ChangeLog 2015-01-12 Thomas Preud'homme thomas.preudhomme@arm.com PR target/64453 * config/arm/arm.c (callee_saved_reg_p): Define. (arm_compute_save_reg0_reg12_mask): Use callee_saved_reg_p to check if register is callee saved instead of !call_used_regs[reg]. (thumb1_compute_save_reg_mask): Likewise. gcc/testsuite/ChangeLog 2014-12-31 Thomas Preud'homme thomas.preudhomme@arm.com * gcc.target/arm/pr64453.c: New. Tested by compiling an arm-none-eabi GCC cross-compiler and running the testsuite on QEMU emulating Cortex-M0 without any regression. Is this ok for trunk? Best regards, Thomas