[Qemu-devel] Questions/comments on TCG

[Stuart Brady]

Hi,

I have a few questions regarding TCG which I'd like to ask, and also a
few minor comments.  I've made a start on a PA-RISC (HPPA) TCG target,
but there are a few things that I'm not sure of.  Before I ask, I should
make it clear that 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.

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.

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).

patch_reloc() takes a 'value' parameter with addend already added -- it
might be preferable for some architectures to pass addend separately, as
this would allow the relocations to be written in a manner which follows
the ABI specification more closely.  On HPPA, the definitions of the
relocations are so twisty, I'd like to avoid any possible thinkos here.
As patch_reloc() is static, this would not slow the other targets down.

The value returned by tcg_target_get_call_iarg_regs_count() is fairly
unsurprisingly used as an upper limit for tcg_target_call_iarg_regs[].
Is there any reason that ARRAY_SIZE(tcg_target_call_iarg_regs) wasn't
used instead?

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.

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.

Perhaps gen_arith would be better placed at the end of tcg_out_op()?

Another possible readability improvement -- the 'tcg_out_' prefix gives
the impression of being part of a defined interface, but target-specific
functions, such as tcg_out_modrm(), share this prefix.  This is made
worse by the fact that every function in tcg-target.c should be declared
static (although some of them aren't).

64-bit SPARC lists ld_i32u_i64 and st_i32_i64 -- the correct names of
these are ld32u_i64, ld32s_i64 and st32_i64.

Again on SPARC, for the arithmetic ops, the constraints used are
{ "r", "0", "rJ" }.  If I've understood correctly, this would mean that
the output can go in any general purpose register, the first operand
must be that same register as the output, and the second operand must be
either another general purpose register, or a 13-bit signed immediate
constant.  In tcg_out_arith(), 'rd', 'r1' and 'r2' can each specifiy a
different register, so should this be { "r", "r", "rJ" }, instead?

There is one register on PA-RISC that I'm not sure whether to reserve:
r19 is call-clobbered, unless position independent code is generated.
If I fail to list r19 in tcg_target_call_clobber_regs, it would be
corrupted when calling non-PIC helpers, but if I do list r19 as a call-
clobbered register, it would be used without being saved when PIC is
generated.  I expect that I can get away with assuming that PIC is not
used, but I'm wondering if it would be better to avoid that assumption.

I will submit patches where I have some idea of what's required, but
I do need to be corrected if there's anything I've misunderstood.

Cheers,
-- 
Stuart Brady