Message ID | CAKdteOadV597TCOe1v3AHoyi9qaKmtgv0XQuEWVuAcmeUqVHDw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Hi, Following our discussions about changing the frame direction on aarch64, I have written a small test case using a large frame which shows similar code generation with and without my patch. There are small differences in register allocation which lead to larger prologue/epilogue with my patch, but I think this could (should) be improved independently. I have attached the sample C source and the code generated with (frane64.s.O2) and without (frame64.s.O2.ref) my patch when compiling at -O2. What do you think? Thanks, Christophe. On 28 June 2013 13:41, Christophe Lyon <christophe.lyon@linaro.org> wrote: > Hi, > Following the discussion on > http://gcc.gnu.org/ml/gcc/2013-05/msg00208.html > here is a patch to change the frame direction. > > Passed 'make check' with no regression on aarch64-none-linux-gnu and > with a few UNSUPPORTED -> PASS on aarch64-none-elf (pr34225, pr38616, > pr38902, pr40971, pr46440, pr47766, pr49307, all of which are guarded > by dg-require-effective-target fstack_protector). > > OK for trunk? > > Thanks, > > Christophe. > 2013-06-28 Christophe Lyon <christophe.lyon@linaro.org> > > * config/aarch64/aarch64.h (FRAME_GROWS_DOWNWARD): Define to 1. > * config/aarch64/aarch64.c (aarch64_initial_elimination_offset): > Add get_frame_size () when eliminating frame pointer.
Ping? The original post is here: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01588.html Thanks, Christophe. On 28 June 2013 13:41, Christophe Lyon <christophe.lyon@linaro.org> wrote: > Hi, > Following the discussion on > http://gcc.gnu.org/ml/gcc/2013-05/msg00208.html > here is a patch to change the frame direction. > > Passed 'make check' with no regression on aarch64-none-linux-gnu and > with a few UNSUPPORTED -> PASS on aarch64-none-elf (pr34225, pr38616, > pr38902, pr40971, pr46440, pr47766, pr49307, all of which are guarded > by dg-require-effective-target fstack_protector). > > OK for trunk? > > Thanks, > > Christophe. > 2013-06-28 Christophe Lyon <christophe.lyon@linaro.org> > > * config/aarch64/aarch64.h (FRAME_GROWS_DOWNWARD): Define to 1. > * config/aarch64/aarch64.c (aarch64_initial_elimination_offset): > Add get_frame_size () when eliminating frame pointer.
On 28/06/13 12:41, Christophe Lyon wrote: > Hi, > Following the discussion on > http://gcc.gnu.org/ml/gcc/2013-05/msg00208.html > here is a patch to change the frame direction. > > Passed 'make check' with no regression on aarch64-none-linux-gnu and > with a few UNSUPPORTED -> PASS on aarch64-none-elf (pr34225, pr38616, > pr38902, pr40971, pr46440, pr47766, pr49307, all of which are guarded > by dg-require-effective-target fstack_protector). > > OK for trunk? > > Thanks, > > Christophe. > 2013-06-28 Christophe Lyon <christophe.lyon@linaro.org> > > * config/aarch64/aarch64.h (FRAME_GROWS_DOWNWARD): Define to 1. > * config/aarch64/aarch64.c (aarch64_initial_elimination_offset): > Add get_frame_size () when eliminating frame pointer. In general, when the files changed are same as the name of the port, it's ok to write: * aarch64.h (...): ... For aarch64_initial_elimination_offset, I suggest "Update offset calculations." Otherwise OK. R. > > > aarch64-frame.txt > > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 527b00d..d8f2801 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1943,7 +1943,7 @@ aarch64_save_or_restore_callee_save_registers (HOST_WIDE_INT offset, > Establish the stack frame by decreasing the stack pointer with a > properly calculated size and, if necessary, create a frame record > filled with the values of LR and previous frame pointer. The > - current FP is also set up is it is in use. */ > + current FP is also set up if it is in use. */ > > void > aarch64_expand_prologue (void) > @@ -4031,7 +4031,7 @@ aarch64_initial_elimination_offset (unsigned from, unsigned to) > return offset - crtl->outgoing_args_size; > > if (from == FRAME_POINTER_REGNUM) > - return cfun->machine->frame.saved_regs_size; > + return cfun->machine->frame.saved_regs_size + get_frame_size (); > } > > if (to == STACK_POINTER_REGNUM) > @@ -4040,6 +4040,7 @@ aarch64_initial_elimination_offset (unsigned from, unsigned to) > { > HOST_WIDE_INT elim = crtl->outgoing_args_size > + cfun->machine->frame.saved_regs_size > + + get_frame_size () > - cfun->machine->frame.fp_lr_offset; > elim = AARCH64_ROUND_UP (elim, STACK_BOUNDARY / BITS_PER_UNIT); > return elim; > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index a08797b..5048258 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -434,7 +434,7 @@ enum reg_class > #define INDEX_REG_CLASS CORE_REGS > #define BASE_REG_CLASS POINTER_REGS > > -/* Register pairs used to eliminate unneeded registers that point intoi > +/* Register pairs used to eliminate unneeded registers that point into > the stack frame. */ > #define ELIMINABLE_REGS \ > { \ > @@ -475,7 +475,7 @@ extern enum aarch64_processor aarch64_tune; > /* Stack layout; function entry, exit and calling. */ > #define STACK_GROWS_DOWNWARD 1 > > -#define FRAME_GROWS_DOWNWARD 0 > +#define FRAME_GROWS_DOWNWARD 1 > > #define STARTING_FRAME_OFFSET 0 > >
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 527b00d..d8f2801 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1943,7 +1943,7 @@ aarch64_save_or_restore_callee_save_registers (HOST_WIDE_INT offset, Establish the stack frame by decreasing the stack pointer with a properly calculated size and, if necessary, create a frame record filled with the values of LR and previous frame pointer. The - current FP is also set up is it is in use. */ + current FP is also set up if it is in use. */ void aarch64_expand_prologue (void) @@ -4031,7 +4031,7 @@ aarch64_initial_elimination_offset (unsigned from, unsigned to) return offset - crtl->outgoing_args_size; if (from == FRAME_POINTER_REGNUM) - return cfun->machine->frame.saved_regs_size; + return cfun->machine->frame.saved_regs_size + get_frame_size (); } if (to == STACK_POINTER_REGNUM) @@ -4040,6 +4040,7 @@ aarch64_initial_elimination_offset (unsigned from, unsigned to) { HOST_WIDE_INT elim = crtl->outgoing_args_size + cfun->machine->frame.saved_regs_size + + get_frame_size () - cfun->machine->frame.fp_lr_offset; elim = AARCH64_ROUND_UP (elim, STACK_BOUNDARY / BITS_PER_UNIT); return elim; diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index a08797b..5048258 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -434,7 +434,7 @@ enum reg_class #define INDEX_REG_CLASS CORE_REGS #define BASE_REG_CLASS POINTER_REGS -/* Register pairs used to eliminate unneeded registers that point intoi +/* Register pairs used to eliminate unneeded registers that point into the stack frame. */ #define ELIMINABLE_REGS \ { \ @@ -475,7 +475,7 @@ extern enum aarch64_processor aarch64_tune; /* Stack layout; function entry, exit and calling. */ #define STACK_GROWS_DOWNWARD 1 -#define FRAME_GROWS_DOWNWARD 0 +#define FRAME_GROWS_DOWNWARD 1 #define STARTING_FRAME_OFFSET 0