Re: [Qemu-devel] [PATCH 05/10] tcg/mips: use stack for TCG temps

[Blue Swirl]

On Sat, Sep 22, 2012 at 5:25 PM, Aurelien Jarno <address@hidden> wrote:
> On Sat, Sep 22, 2012 at 02:37:35PM +0000, Blue Swirl wrote:
>> On Fri, Sep 21, 2012 at 4:43 PM, Aurelien Jarno <address@hidden> wrote:
>> > Use stack instead of temp_buf array in CPUState for TCG
>> > temps.
>> >
>> > Signed-off-by: Aurelien Jarno <address@hidden>
>> > ---
>> >  tcg/mips/tcg-target.c |   10 ++++++----
>> >  1 file changed, 6 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
>> > index 0ea6a76..c05169f 100644
>> > --- a/tcg/mips/tcg-target.c
>> > +++ b/tcg/mips/tcg-target.c
>> > @@ -1538,11 +1538,15 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>> >  {
>> >      int i, frame_size;
>> >
>> > -    /* reserve some stack space */
>> > +    /* reserve some stack space, also for TCG temps. */
>> >      frame_size = ARRAY_SIZE(tcg_target_callee_save_regs) * 4
>> > -                 + TCG_STATIC_CALL_ARGS_SIZE;
>> > +                 + TCG_STATIC_CALL_ARGS_SIZE
>> > +                 + CPU_TEMP_BUF_NLONGS * sizeof(long);
>> >      frame_size = (frame_size + TCG_TARGET_STACK_ALIGN - 1) &
>> >                   ~(TCG_TARGET_STACK_ALIGN - 1);
>> > +    tcg_set_frame(s, TCG_REG_SP, ARRAY_SIZE(tcg_target_callee_save_regs) 
>> > * 4
>> > +                  + TCG_STATIC_CALL_ARGS_SIZE,
>> > +                  CPU_TEMP_BUF_NLONGS * sizeof(long));
>>
>> My version used frame_size instead of duplicating a part of the
>> calculations, wouldn't that take stack alignment also in
>> consideration?
>>
>> http://lists.nongnu.org/archive/html/qemu-devel/2011-06/msg02566.html
>
> Oh haven't seen this patch before. Well the choice is basically between
> adding one value and subtracting one, not sure what is the best.
>
> About the alignment, do we require some specific alignment for the
> temps? As long as they are aligned wrt size, I think it's fine.

Well, since we align the frame, that could be a better reference.
Usually stack doesn't get unaligned but why is there then the
alignment calculation?

>
>> >
>> >      /* TB prologue */
>> >      tcg_out_addi(s, TCG_REG_SP, -frame_size);
>> > @@ -1597,6 +1601,4 @@ static void tcg_target_init(TCGContext *s)
>> >      tcg_regset_set_reg(s->reserved_regs, TCG_REG_GP);   /* global pointer 
>> > */
>> >
>> >      tcg_add_target_add_op_defs(mips_op_defs);
>> > -    tcg_set_frame(s, TCG_AREG0, offsetof(CPUArchState, temp_buf),
>> > -                  CPU_TEMP_BUF_NLONGS * sizeof(long));
>> >  }
>> > --
>> > 1.7.10.4
>> >
>> >
>>
>
> --
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> address@hidden                 http://www.aurel32.net