* [Qemu-devel] Questions/comments on TCG
@ 2008-03-07 12:37 Stuart Brady
2008-03-07 16:07 ` Blue Swirl
2008-03-07 18:28 ` [Qemu-devel] [PATCH] Remove blank elements in tcg_target_reg_alloc_order[] Stuart Brady
0 siblings, 2 replies; 8+ messages in thread
From: Stuart Brady @ 2008-03-07 12:37 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 12:37 [Qemu-devel] Questions/comments on TCG Stuart Brady
@ 2008-03-07 16:07 ` Blue Swirl
2008-03-07 18:19 ` Stuart Brady
2008-03-07 18:28 ` [Qemu-devel] [PATCH] Remove blank elements in tcg_target_reg_alloc_order[] Stuart Brady
1 sibling, 1 reply; 8+ messages in thread
From: Blue Swirl @ 2008-03-07 16:07 UTC (permalink / raw)
To: qemu-devel
On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> 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.
Well, I'd still recommend using x86 as a reference until Sparc works
or you may copy a faulty design.
> 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.
> 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?
> 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.
I'm not sure about patch_reloc use. On Sparc there are other
unimplemented relocations that should be much more common, but TCG
never aborts in patch_reloc.
> 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?
Good question. There are also other uses of arrays where NULL is used
as a terminator when the size would always be known anyway.
> 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.
> 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.
> Perhaps gen_arith would be better placed at the end of tcg_out_op()?
Right.
> 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.
Yes, I'll fix these.
> 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?
Oh, that's what the '0' means. I'll try your version.
> 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.
Maybe tcg_target_init should take a flag parameter?
> 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.
I'm not sure if I have understood everything correctly either.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 16:07 ` Blue Swirl
@ 2008-03-07 18:19 ` Stuart Brady
2008-03-07 18:47 ` Blue Swirl
0 siblings, 1 reply; 8+ messages in thread
From: Stuart Brady @ 2008-03-07 18:19 UTC (permalink / raw)
To: qemu-devel
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.
> > 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.
> > 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.
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.
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] Remove blank elements in tcg_target_reg_alloc_order[]
2008-03-07 12:37 [Qemu-devel] Questions/comments on TCG Stuart Brady
2008-03-07 16:07 ` Blue Swirl
@ 2008-03-07 18:28 ` Stuart Brady
1 sibling, 0 replies; 8+ messages in thread
From: Stuart Brady @ 2008-03-07 18:28 UTC (permalink / raw)
To: qemu-devel
Hi,
tcg_target_reg_alloc_order[] contains blank elements, which default to
TCG_REG_EAX/TCG_REG_RAX on i386/x86_64, and TCG_REG_G0 on SPARC. The
included patch removes these elements, and adds an ARRAY_SIZE macro to
osdep.h, which is then used to check the size of the array.
I'm not sure if the addition of ARRAY_SIZE is at all controversial.
I'll happily resubmit the patch with this removed if it's disliked.
Cheers,
--
Stuart Brady
diff -urp qemu-orig/osdep.h qemu-new/osdep.h
--- qemu-orig/osdep.h 2008-01-31 19:42:53.000000000 +0000
+++ qemu-new/osdep.h 2008-03-07 15:44:49.000000000 +0000
@@ -26,6 +26,10 @@
#define MAX(a, b) (((a) > (b)) ? (a) : (b))
#endif
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
#ifndef always_inline
#if (__GNUC__ < 3) || defined(__APPLE__)
#define always_inline inline
diff -urp qemu-orig/tcg/i386/tcg-target.c qemu-new/tcg/i386/tcg-target.c
--- qemu-orig/tcg/i386/tcg-target.c 2008-02-10 16:50:28.000000000 +0000
+++ qemu-new/tcg/i386/tcg-target.c 2008-03-07 15:26:47.000000000 +0000
@@ -32,7 +32,7 @@ const char *tcg_target_reg_names[TCG_TAR
"%edi",
};
-int tcg_target_reg_alloc_order[TCG_TARGET_NB_REGS] = {
+int tcg_target_reg_alloc_order[] = {
TCG_REG_EAX,
TCG_REG_EDX,
TCG_REG_ECX,
diff -urp qemu-orig/tcg/sparc/tcg-target.c qemu-new/tcg/sparc/tcg-target.c
--- qemu-orig/tcg/sparc/tcg-target.c 2008-03-02 15:09:47.000000000 +0000
+++ qemu-new/tcg/sparc/tcg-target.c 2008-03-07 15:27:29.000000000 +0000
@@ -57,7 +57,7 @@ static const char * const tcg_target_reg
"%i7",
};
-static const int tcg_target_reg_alloc_order[TCG_TARGET_NB_REGS] = {
+static const int tcg_target_reg_alloc_order[] = {
TCG_REG_L0,
TCG_REG_L1,
TCG_REG_L2,
diff -urp qemu-orig/tcg/tcg.c qemu-new/tcg/tcg.c
--- qemu-orig/tcg/tcg.c 2008-02-19 01:21:18.000000000 +0000
+++ qemu-new/tcg/tcg.c 2008-03-07 15:26:23.000000000 +0000
@@ -1212,14 +1212,14 @@ static int tcg_reg_alloc(TCGContext *s,
tcg_regset_andnot(reg_ct, reg1, reg2);
/* first try free registers */
- for(i = 0; i < TCG_TARGET_NB_REGS; i++) {
+ for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
reg = tcg_target_reg_alloc_order[i];
if (tcg_regset_test_reg(reg_ct, reg) && s->reg_to_temp[reg] == -1)
return reg;
}
/* XXX: do better spill choice */
- for(i = 0; i < TCG_TARGET_NB_REGS; i++) {
+ for(i = 0; i < ARRAY_SIZE(tcg_target_reg_alloc_order); i++) {
reg = tcg_target_reg_alloc_order[i];
if (tcg_regset_test_reg(reg_ct, reg)) {
tcg_reg_free(s, reg);
diff -urp qemu-orig/tcg/x86_64/tcg-target.c qemu-new/tcg/x86_64/tcg-target.c
--- qemu-orig/tcg/x86_64/tcg-target.c 2008-03-02 15:09:47.000000000 +0000
+++ qemu-new/tcg/x86_64/tcg-target.c 2008-03-07 15:27:14.000000000 +0000
@@ -40,7 +40,7 @@ const char *tcg_target_reg_names[TCG_TAR
"%r15",
};
-int tcg_target_reg_alloc_order[TCG_TARGET_NB_REGS] = {
+int tcg_target_reg_alloc_order[] = {
TCG_REG_RDI,
TCG_REG_RSI,
TCG_REG_RDX,
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 18:19 ` Stuart Brady
@ 2008-03-07 18:47 ` Blue Swirl
2008-03-07 19:55 ` Stuart Brady
0 siblings, 1 reply; 8+ messages in thread
From: Blue Swirl @ 2008-03-07 18:47 UTC (permalink / raw)
To: qemu-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 18:47 ` Blue Swirl
@ 2008-03-07 19:55 ` Stuart Brady
2008-03-07 20:30 ` Blue Swirl
0 siblings, 1 reply; 8+ messages in thread
From: Stuart Brady @ 2008-03-07 19:55 UTC (permalink / raw)
To: qemu-devel
On Fri, Mar 07, 2008 at 08:47:03PM +0200, Blue Swirl wrote:
> On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> > 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.
Possibly not, as g0 is marked as reserved, but it looks to me like bug,
regardless of whether it causes any harm, so I've submitted a patch.
> > 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.
Right, op_call does need to link, and that clobbers the link register,
so it must be restored -- but I've a feeling that this isn't happening.
I expect you could copy o7 to/from i5 before/after the call (or jmpl)...
although I'm not sure if you'd also need to save the frame pointer.
op_exit_tb also needs the link register, as it needs to know where to
return to. I see you've fixed op_goto_tb -- I'm not sure if TCG_REG_I5
now needs removing from tcg_target_reg_alloc_order[], though.
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 19:55 ` Stuart Brady
@ 2008-03-07 20:30 ` Blue Swirl
2008-03-08 14:07 ` Blue Swirl
0 siblings, 1 reply; 8+ messages in thread
From: Blue Swirl @ 2008-03-07 20:30 UTC (permalink / raw)
To: qemu-devel
On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> On Fri, Mar 07, 2008 at 08:47:03PM +0200, Blue Swirl wrote:
> > On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
>
> > > 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.
>
>
> Possibly not, as g0 is marked as reserved, but it looks to me like bug,
> regardless of whether it causes any harm, so I've submitted a patch.
>
>
> > > 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.
>
>
> Right, op_call does need to link, and that clobbers the link register,
> so it must be restored -- but I've a feeling that this isn't happening.
> I expect you could copy o7 to/from i5 before/after the call (or jmpl)...
> although I'm not sure if you'd also need to save the frame pointer.
Another possibility is to add function epilogue with save and add
restore to ret (or use v9 return).
> op_exit_tb also needs the link register, as it needs to know where to
> return to. I see you've fixed op_goto_tb -- I'm not sure if TCG_REG_I5
> now needs removing from tcg_target_reg_alloc_order[], though.
Good point, better remove it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Questions/comments on TCG
2008-03-07 20:30 ` Blue Swirl
@ 2008-03-08 14:07 ` Blue Swirl
0 siblings, 0 replies; 8+ messages in thread
From: Blue Swirl @ 2008-03-08 14:07 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
On 3/7/08, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> > On Fri, Mar 07, 2008 at 08:47:03PM +0200, Blue Swirl wrote:
> > > On 3/7/08, Stuart Brady <sdbrady@ntlworld.com> wrote:
> >
> > > > 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.
> >
> >
> > Possibly not, as g0 is marked as reserved, but it looks to me like bug,
> > regardless of whether it causes any harm, so I've submitted a patch.
> >
> >
> > > > 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.
> >
> >
> > Right, op_call does need to link, and that clobbers the link register,
> > so it must be restored -- but I've a feeling that this isn't happening.
> > I expect you could copy o7 to/from i5 before/after the call (or jmpl)...
> > although I'm not sure if you'd also need to save the frame pointer.
>
>
> Another possibility is to add function epilogue with save and add
> restore to ret (or use v9 return).
I added the save and restore instructions, because if the generated
code made any calls, the registers were overwritten.
Currently on Sparc64 host a small helloworld program executes until
the system call, then Qemu dies with illegal instruction. It looks
like this is caused by setjmp/longjmp register mangling bugs in Linux
glibc, my workaround does not help. I'd be interested to hear if this
works any better on Solaris/Sparc or *BSD/Sparc. On Sparc32 TB linking
does not work, so Qemu dies on TB switch.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: helloworld.c --]
[-- Type: text/x-csrc; name=helloworld.c, Size: 267 bytes --]
#define __KERNEL__
#include <asm/unistd.h>
static int errno;
static __inline__ _syscall1(void,exit,int,exitval)
static inline _syscall3(int,write,int,fd,const char *,buf,long,count)
int _start()
{
write(2, "Hello World!\n", sizeof("Hello World!\n"));
exit(0);
}
[-- Attachment #3: helloworld.sparc32 --]
[-- Type: application/octet-stream, Size: 712 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-03-08 14:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-07 12:37 [Qemu-devel] Questions/comments on TCG Stuart Brady
2008-03-07 16:07 ` Blue Swirl
2008-03-07 18:19 ` Stuart Brady
2008-03-07 18:47 ` Blue Swirl
2008-03-07 19:55 ` Stuart Brady
2008-03-07 20:30 ` Blue Swirl
2008-03-08 14:07 ` Blue Swirl
2008-03-07 18:28 ` [Qemu-devel] [PATCH] Remove blank elements in tcg_target_reg_alloc_order[] Stuart Brady
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).