RE: [PATCH, ARM] Fix PR64453: live high register not saved in function prolog with -Os
Hi Ramana, > From: Ramana Radhakrishnan [mailto:ramana@googlemail.com] > Sent: Wednesday, January 14, 2015 7:21 PM > On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > 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.preudho...@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.preudho...@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
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 wrote: > Hi Ramana, > >> From: Ramana Radhakrishnan [mailto:ramana@googlemail.com] >> Sent: Wednesday, January 14, 2015 7:21 PM >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme >> 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.preudho...@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.preudho...@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 > > >
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@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 > wrote: > > Hi Ramana, > > > >> From: Ramana Radhakrishnan [mailto:ramana@googlemail.com] > >> Sent: Wednesday, January 14, 2015 7:21 PM > >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > >> 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.preudho...@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.preudho...@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 > > > > > >
RE: [PATCH, ARM] Fix PR64453: live high register not saved in function prolog with -Os
And I forgot to update the date in the rush so I committed the fix as obvious: diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ed8ca31..809f5cf 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,4 +1,4 @@ -2015-01-23 Thomas Preud'homme +2015-03-03 Thomas Preud'homme Backport from mainline 2015-01-14 Thomas Preud'homme diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 40b2ac5..bb5d545 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,4 +1,4 @@ -2015-01-23 Thomas Preud'homme +2015-03-03 Thomas Preud'homme Backport from mainline 2015-01-14 Thomas Preud'homme Sorry for the mistake. Best regards, Thomas > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@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@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 > > wrote: > > > Hi Ramana, > > > > > >> From: Ramana Radhakrishnan [mailto:ramana@googlemail.com] > > >> Sent: Wednesday, January 14, 2015 7:21 PM > > >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > > >> 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.preudho...@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.preudho...@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 > > > > > > > > > > > > >
RE: [PATCH, ARM] Fix PR64453: live high register not saved in function prolog with -Os
Done for backport to 4.8. Best regards, Thomas > -Original Message- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@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@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 > > wrote: > > > Hi Ramana, > > > > > >> From: Ramana Radhakrishnan [mailto:ramana@googlemail.com] > > >> Sent: Wednesday, January 14, 2015 7:21 PM > > >> On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme > > >> 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.preudho...@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.preudho...@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 > > > > > > > > > > > > >
Re: [PATCH, ARM] Fix PR64453: live high register not saved in function prolog with -Os
On Wed, Jan 14, 2015 at 10:20 AM, Thomas Preud'homme 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.preudho...@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.preudho...@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 000..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