Re: [Qemu-devel] Questions/comments on TCG

["Blue Swirl" <blauwirbel@gmail.com>]

On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> On Fri, Mar 07, 2008 at 06:07:32PM +0200, Blue Swirl wrote:
>  > On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
>
> > >  I do understand that the current SPARC TCG code is preliminary work.
>
> > >  However, in some ways, I feel it still serves as a better reference than
>  > >  i386 and x86_64
>  >
>  > Well, I'd still recommend using x86 as a reference until Sparc works
>  > or you may copy a faulty design.
>
>
> Don't worry -- I still checked with the x86 targets.  I only really
>  needed a quick idea of what was required for a new target.
>
>
>  > >  Which registers should go in tcg_target_reg_alloc_order[]?  I notice
>  > >  that i386 includes ESP, which tcg_target_init() marks as reserved, and
>  > >  x86_64 includes RBX and RBP, which are again marked as reserved.
>  >
>  > I put there only the registers that should be safe to use, the G
>  > registers may have issues or they are already used as global
>  > registers. Also we should not need frame pointer.
>
>
> Sounds reasonable.  I think I really meant to ask what _shouldn't_ go in
>  tcg_target_reg_alloc_order[].  I was mainly confused by the inclusion of
>  registers which are marked as reserved on the x86 targets.
>
>
>  > >  Furthermore, x86_64's tcg_target_reg_alloc_order[] contains 16 elements
>  > >  (TCG_TARGET_NB_REGS), but only 15 are specified -- the last element is
>  > >  left as 0, which is TCG_REG_RAX.  SPARC also does this, but with
>  > >  TARGET_REG_G0 (which is marked as reserved, as it's hardwired to zero).
>  >
>  > Maybe I missed something, but g0 isn't in the reg_alloc_order?
>
>
> tcg_target_reg_alloc_order[] has 32 elements, but only 14 are used.
>  The rest hold 0, specifying TCG_REG_G0.

I see. That could be asking for trouble.

>  > >  On SPARC, I notice that goto_tb is handled using CALL and JMPL, placing
>  > >  the return address in o7... but we're returning from a TB, or jumping to
>  > >  another one, so surely we shouldn't link here?  Also, TCG_TYPE_TL is
>  > >  used for exit_tb's return value, I think this should be the host's long
>  > >  (using TCG_TYPE_PTR) instead.
>  >
>  > These are bugs, thanks for spotting. I was using o7 if a register is
>  > needed, it will be clobbered anyway.
>
>
> I don't understand -- o7 is required when returning in exit_tb, so if it
>  is used, it must be saved and restored.

Not exit_tb, but call.

>  > >  Also on SPARC, could the indentation of the OP_32_64s be improved?
>  > >  Yeah, it's not a serious problem, but I feel it would make the code
>  > >  slightly easier to read.
>  >
>  > It's not my fault, Emacs wants to do it this way. I'm open to your
>  > suggestions.
>
>
> Oh dear, I'm such a vim user, I don't even have Emacs installed. :)
>
>  How about something like this?
>
>  #if defined(__sparc_v9__) && !defined(__sparc_v8plus__)
>  #define OP_32_64(x)                             \
>         glue(glue(case INDEX_op_, x), _i32:)    \
>         glue(glue(case INDEX_op_, x), _i64)
>  #else
>  #define OP_32_64(x)                             \
>         glue(glue(case INDEX_op_, x), _i32)
>  #endif
>  ...
>     OP_32_64(ld8u):
>         tcg_out_ldst(s, args[0], args[1], args[2], LDUB);
>         break;
>  ...
>
>  The macro might be a bit sick, but hopefully it would make Emacs happy,
>  and I feel ':' does make a certain amount of sense, here.

Unfortunately it confuses Emacs even more.

>  It probably wouldn't help with indentation, but you could always do
>  something like this:
>
>  #if defined(__sparc_v9__) && !defined(__sparc_v8plus__)
>  #define v9(x) x
>  #else
>  #define v9(x)
>  #endif
>  ...
>     case INDEX_op_ld8u_i32:
>  v9( case INDEX_op_ld8u_i64: )
>         tcg_out_ldst(s, args[0], args[1], args[2], LDUB);
>         break;
>  ...
>
>  I'll admit, that looks unusual, but it would avoid breaking searches for
>  ld8u_i32 or op_ld8u.

This also does not work.