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
