Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host
Am 06.08.2011 01:17, schrieb Kenneth Salerno:
With this patch it gets caught up in tcg/tcg.c line 1646:
if (ts->val_type == TEMP_VAL_REG)
...
else if (ts->val_type == TEMP_VAL_MEM)
...
else if (ts->val_type == TEMP_VAL_CONST)
} else { <--- we get here by changing unsigned int to unsigned
character
tcg_abort();
}
Output from QEMU:
/home/kens/cross-compile/qemu/testing/qemu/tcg/tcg.c:1646: tcg fatal error
This application has requested the Runtime to terminate it in an
unusual way.
Please contact the application's support team for more information.
Thanks,
Ken
That's a different issue. Read more here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00758.html
http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00797.html
Regards,
Stefan
Re: [Qemu-devel] another TCG branch weirdness
On Fri, Aug 5, 2011 at 10:21 PM, Artyom Tarasenko wrote:
> On Fri, Aug 5, 2011 at 10:32 PM, Blue Swirl wrote:
>> On Fri, Aug 5, 2011 at 4:36 PM, Artyom Tarasenko wrote:
>>> Host x86_64, guest sparc64. Found a case where a branch instruction
>>> (brz,pn %o0) unexpectedly jumps to an unexpected address. I.e.
>>> branch shouldn't be taken at all, but even if it were it should have
>>> been to 0x13e26e4 and not to 0x5.
>>>
>>> Was about to write that the generated OP for brz,pn usually looks
>>> different, when realized that in fact it was even generated for this
>>> very address just before, but with another branch in the delay slot.
>>> The bug looks familiar, Blue, isn't it? :)
>>
>> Sorry, does not ring a bell.
>
> I meant c27e275 where you fixed unconditional branch in a delay slot.
> (One of my first bug reports).
> Now it looks pretty similar for the conditional branches.
>
>>> IN:
>>> 0x013e26c0: brz,pn %o0, 0x13e26e4
>>> 0x013e26c4: brlez,pn %o1, 0x13e26e4
>>>
>>> OP:
>>> 0x13e26c0
>>> ld_i64 tmp6,regwptr,$0x0
>>> movi_i64 cond,$0x0
>>> movi_i64 tmp8,$0x0
>>> brcond_i64 tmp6,tmp8,ne,$0x0
>>> movi_i64 cond,$0x1
>>> set_label $0x0
>>>
>>> ^^^ Ok, that's how brz,pn usually looks like
>>>
>>> 0x13e26c4
>>> ld_i64 tmp7,regwptr,$0x8
>>> movi_i64 tmp8,$0x0
>>> brcond_i64 cond,tmp8,eq,$0x1
>>> movi_i64 npc,$0x13e26e4
>>> br $0x2
>>> set_label $0x1
>>> movi_i64 npc,$0x13e26c8
>>> set_label $0x2
>>> movi_i64 cond,$0x0
>>> movi_i64 tmp8,$0x0
>>> brcond_i64 tmp7,tmp8,gt,$0x3
>>> movi_i64 cond,$0x1
>>> set_label $0x3
>>> movi_i64 tmp0,$0x0
>>> brcond_i64 cond,tmp0,eq,$0x4
>>> movi_i64 npc,$0x13e26e4
>>> br $0x5
>>> set_label $0x4
>>> movi_i64 npc,$0x5
>>> set_label $0x5
>>> exit_tb $0x0
>>> --
>>> IN:
>>> 0x013e26c0: brz,pn %o0, 0x13e26e4
>>>
>>> OP:
>>> 0x13e26c0
>>> ld_i64 tmp6,regwptr,$0x0
>>> movi_i64 cond,$0x0
>>> movi_i64 tmp8,$0x0
>>> brcond_i64 tmp6,tmp8,ne,$0x0
>>> movi_i64 cond,$0x1
>>> set_label $0x0
>>> movi_i64 pc,$0x5
>>>
>>> ^^^ What's that?
>>
>> Probably DYNAMIC_PC + 4. I guess we are hitting this ancient comment
>> in target-sparc/translate.c:1372:
>> /* XXX: potentially incorrect if dynamic npc */
>
> Yes, I think this too. The following patch passes my tests. Do you
> think it's correct? If yes, I'll make it for the other branches too.
Looks OK. All these almost identical checks are a worrying: are all
cases covered? Is the logic same when it should be? Perhaps there
should be centralized handling, for example gen_next_pc_branch()
gen_next_pc_delay_slot() etc. with asserts.
Also reusing dc->pc etc for in band signaling is not robust as this case shows.
> @@ -1384,8 +1399,14 @@ static void do_branch_reg(DisasContext *dc,
> int32_t offset, uint32_t insn,
> } else {
> dc->pc = dc->npc;
> dc->jump_pc[0] = target;
> - dc->jump_pc[1] = dc->npc + 4;
> - dc->npc = JUMP_PC;
> + if (unlikely(dc->npc == DYNAMIC_PC)) {
> + dc->jump_pc[1] = DYNAMIC_PC;
> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> +
> + } else {
> + dc->jump_pc[1] = dc->npc + 4;
> + dc->npc = JUMP_PC;
> + }
>
>
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>
Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host
On 08/05/2011 10:25 PM, TeLeMan wrote:
On Sat, Aug 6, 2011 at 04:46, Blue Swirl wrote:
On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
wrote:
Hi,
I'm not sure if any defaults (build or runtime) have changed since 0.14.1, but
I can no longer get the following to work anymore for QEMU versions 0.15.0-rc2
or recent development builds:
-device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
Works great in 0.14.1 however.
From the QEMU console, "info networking" shows the NIC e1000 and the VLAN
correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized and networking setup,
just can't seem to communicate with the gateway (10.0.2.2). The only difference I see in
the console is cosmetic (restricted=off rather than restricted=n).
Host OS: Windows XP
Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
mingw32-gettext 0.18.1-2
Guest OS: RHEL 6.1
Is it just me?
No, this is fallout from glib use:
http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
The fix is to rewrite structures without using GCC bit fields.
-mms-bitfields affects all byte-alignments in a structure. For example,
struct s
{
uint8_t a;
uint32_t b;
} __attribute__((packed));
sizeof(s) is 5 without -mms-bitfields but sizeof(s) is 8 with -mms-bitfields.
If you can identify the offending structs, you can do:
#pragma pack(push,1)
struct s
{
uint8_t a;
uint32_t b;
} __attribute__((packed));
#pragma pack(pop)
Regards,
Anthony Liguori
[Qemu-devel] [PATCH] TCG: fix copy propagation
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 13 +++--
tcg/tcg.h |5 +
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..748ecf9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,13 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
-int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
-if (src >= nb_globals) {
+if (src >= nb_globals &&
+tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
if (temps[src].state != TCG_TEMP_HAS_COPY) {
temps[src].state = TCG_TEMP_HAS_COPY;
@@ -474,7 +475,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 3;
@@ -500,7 +501,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1], nb_temps,
+tcg_opt_gen_mov(s, gen_args, args[0], args[1], nb_temps,
nb_globals);
gen_args += 2;
args += 3;
@@ -523,7 +524,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
break;
}
if (temps[args[1]].state != TCG_TEMP_CONST) {
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 2;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index e76f9af..e2a7095 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -410,6 +410,11 @@ static inline TCGv_i64 tcg_temp_local_new_i64(void)
void tcg_temp_free_i64(TCGv_i64 arg);
char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size,
TCGv_i64 arg);
+static inline bool tcg_arg_is_local(TCGContext *s, TCGArg arg)
+{
+return s->temps[arg].temp_local;
+}
+
#if defined(CONFIG_DEBUG_TCG)
/* If you call tcg_clear_temp_count() at the start of a section of
* code which is not supposed to leak any TCG temporaries, then
--
1.6.2.4
From f8f16733f469eebae8f6132abc70e7357e41cf7f Mon Sep 17 00:00:00 2001
Message-Id:
From: Blue Swirl
Date: Sat, 6 Aug 2011 13:58:47 +
Subject: [PATCH] TCG: fix copy propagation
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 13 +++--
tcg/tcg.h |5 +
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..748ecf9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,13 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
-int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
-if (src >= nb_globals) {
+if (src >= nb_globals &&
+tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
if (temps[src].state != TCG_TEMP_HAS_COPY) {
temps[src].state = TCG_TEMP_HAS_COPY;
@@ -474,7 +475,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host
On Sat, Aug 6, 2011 at 1:33 PM, Anthony Liguori wrote:
> On 08/05/2011 10:25 PM, TeLeMan wrote:
>>
>> On Sat, Aug 6, 2011 at 04:46, Blue Swirl wrote:
>>>
>>> On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
>>> wrote:
Hi,
I'm not sure if any defaults (build or runtime) have changed since
0.14.1, but I can no longer get the following to work anymore for QEMU
versions 0.15.0-rc2 or recent development builds:
-device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
Works great in 0.14.1 however.
From the QEMU console, "info networking" shows the NIC e1000 and the
VLAN correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized
and
networking setup, just can't seem to communicate with the gateway
(10.0.2.2). The only difference I see in the console is cosmetic
(restricted=off rather than restricted=n).
Host OS: Windows XP
Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
mingw32-gettext 0.18.1-2
Guest OS: RHEL 6.1
Is it just me?
>>>
>>> No, this is fallout from glib use:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
>>>
>>> The fix is to rewrite structures without using GCC bit fields.
>>
>> -mms-bitfields affects all byte-alignments in a structure. For example,
>> struct s
>> {
>> uint8_t a;
>> uint32_t b;
>> } __attribute__((packed));
>>
>> sizeof(s) is 5 without -mms-bitfields but sizeof(s) is 8 with
>> -mms-bitfields.
>
> If you can identify the offending structs, you can do:
>
> #pragma pack(push,1)
>
> struct s
> {
> uint8_t a;
> uint32_t b;
> } __attribute__((packed));
>
> #pragma pack(pop)
I grepped the tree for ((packed)). The only two places where bit
fields are used with packed structs are in SLIRP:
struct ip {
#ifdef HOST_WORDS_BIGENDIAN
u_int ip_v:4, /* version */
ip_hl:4;/* header length */
#else
u_int ip_hl:4, /* header length */
ip_v:4; /* version */
#endif
uint8_t ip_tos; /* type of service */
uint16_tip_len; /* total length */
uint16_tip_id; /* identification */
uint16_tip_off; /* fragment offset field */
#define IP_DF 0x4000/* don't fragment flag */
#define IP_MF 0x2000/* more fragments flag */
#define IP_OFFMASK 0x1fff /* mask for fragmenting bits */
uint8_t ip_ttl; /* time to live */
uint8_t ip_p; /* protocol */
uint16_tip_sum; /* checksum */
struct in_addr ip_src,ip_dst; /* source and dest address */
} __attribute__((packed));
struct ip_timestamp {
uint8_t ipt_code; /* IPOPT_TS */
uint8_t ipt_len;/* size of structure (variable) */
uint8_t ipt_ptr;/* index of current entry */
#ifdef HOST_WORDS_BIGENDIAN
u_int ipt_oflw:4, /* overflow counter */
ipt_flg:4; /* flags, see below */
#else
u_int ipt_flg:4, /* flags, see below */
ipt_oflw:4; /* overflow counter */
#endif
union ipt_timestamp {
n_long ipt_time[1];
struct ipt_ta {
struct in_addr ipt_addr;
n_long ipt_time;
} ipt_ta[1];
} ipt_timestamp;
} __attribute__((packed));
I'd avoid the bit fields altogether in both cases, then also the
#ifdeffery could be removed.
Re: [Qemu-devel] [PATCH 2/3] Introduce a 'client_add' monitor command accepting an open FD
On 06/23/2011 07:31 AM, Daniel P. Berrange wrote:
Allow client connections for VNC and socket based character
devices to be passed in over the monitor using SCM_RIGHTS.
One intended usage scenario is to start QEMU with VNC on a
UNIX domain socket. An unprivileged user which cannot access
the UNIX domain socket, can then connect to QEMU's VNC server
by passing an open FD to libvirt, which passes it onto QEMU.
{ "execute": "get_fd", "arguments": { "fdname": "myclient" } }
{ "return": {} }
{ "execute": "add_client", "arguments": { "protocol": "vnc",
"fdname": "myclient",
"skipauth": true } }
{ "return": {} }
In this case 'protocol' can be 'vnc' or 'spice', or the name
of a character device (eg from -chardev id=)
The 'skipauth' parameter can be used to skip any configured
VNC authentication scheme, which is useful if the mgmt layer
talking to the monitor has already authenticated the client
in another way.
* console.h: Define 'vnc_display_add_client' method
* monitor.c: Implement 'client_add' command
* qemu-char.c, qemu-char.h: Add 'qemu_char_add_client' method
I don't feel all that good about this anymore. The notion of adding a
fd to an arbitrary character device is a big layering violation and
doesn't make much sense conceptually.
A chardev cannot have multiple connections. It seems like the use-case
is much better suited to having an fd: protocol to create char devices with.
I'd like to partially revert this removing the qemu_chr_add_client
interface.
Regards,
Anthony Liguori
* qerror.c, qerror.h: Add QERR_ADD_CLIENT_FAILED
* qmp-commands.hx: Declare 'client_add' command
* ui/vnc.c: Implement 'vnc_display_add_client' method
---
console.h |1 +
monitor.c | 32
qemu-char.c | 30 --
qemu-char.h |2 ++
qerror.c|4
qerror.h|3 +++
qmp-commands.hx | 27 +++
ui/vnc.c|7 +++
8 files changed, 100 insertions(+), 6 deletions(-)
diff --git a/console.h b/console.h
index 64d1f09..84d3e8d 100644
--- a/console.h
+++ b/console.h
@@ -372,6 +372,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
void vnc_display_init(DisplayState *ds);
void vnc_display_close(DisplayState *ds);
int vnc_display_open(DisplayState *ds, const char *display);
+void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
int vnc_display_disable_login(DisplayState *ds);
char *vnc_display_local_addr(DisplayState *ds);
#ifdef CONFIG_VNC
diff --git a/monitor.c b/monitor.c
index 6af6a4d..23c310e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1185,6 +1185,38 @@ static int expire_password(Monitor *mon, const QDict
*qdict, QObject **ret_data)
return -1;
}
+static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject
**ret_data)
+{
+const char *protocol = qdict_get_str(qdict, "protocol");
+const char *fdname = qdict_get_str(qdict, "fdname");
+int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
+CharDriverState *s;
+
+if (strcmp(protocol, "spice") == 0) {
+if (!using_spice) {
+/* correct one? spice isn't a device ,,, */
+qerror_report(QERR_DEVICE_NOT_ACTIVE, "spice");
+return -1;
+}
+ qerror_report(QERR_ADD_CLIENT_FAILED);
+ return -1;
+} else if (strcmp(protocol, "vnc") == 0) {
+ int fd = monitor_get_fd(mon, fdname);
+ vnc_display_add_client(NULL, fd, skipauth);
+ return 0;
+} else if ((s = qemu_chr_find(protocol)) != NULL) {
+ int fd = monitor_get_fd(mon, fdname);
+ if (qemu_chr_add_client(s, fd)< 0) {
+ qerror_report(QERR_ADD_CLIENT_FAILED);
+ return -1;
+ }
+ return 0;
+}
+
+qerror_report(QERR_INVALID_PARAMETER, "protocol");
+return -1;
+}
+
static int client_migrate_info(Monitor *mon, const QDict *qdict, QObject
**ret_data)
{
const char *protocol = qdict_get_str(qdict, "protocol");
diff --git a/qemu-char.c b/qemu-char.c
index fb13b28..4e168ee 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -168,6 +168,11 @@ int qemu_chr_get_msgfd(CharDriverState *s)
return s->get_msgfd ? s->get_msgfd(s) : -1;
}
+int qemu_chr_add_client(CharDriverState *s, int fd)
+{
+return s->chr_add_client ? s->chr_add_client(s, fd) : -1;
+}
+
void qemu_chr_accept_input(CharDriverState *s)
{
if (s->chr_accept_input)
@@ -2123,6 +2128,22 @@ static void socket_set_nodelay(int fd)
setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (char *)&val, sizeof(val));
}
+static int tcp_chr_add_client(CharDriverState *chr, int fd)
+{
+TCPCharDriver *s = chr->opaque;
+if (s->fd != -1)
+ return -1;
+
+socket_set_nonblock(fd);
+if (s->do_nodelay)
+socket_set_nodelay(fd);
+s->fd = fd;
+qemu_set_fd_handler(s->listen_fd
Re: [Qemu-devel] another TCG branch weirdness
On Sat, Aug 6, 2011 at 2:09 PM, Blue Swirl wrote:
> On Fri, Aug 5, 2011 at 10:21 PM, Artyom Tarasenko wrote:
>> On Fri, Aug 5, 2011 at 10:32 PM, Blue Swirl wrote:
>>> On Fri, Aug 5, 2011 at 4:36 PM, Artyom Tarasenko
>>> wrote:
Host x86_64, guest sparc64. Found a case where a branch instruction
(brz,pn %o0) unexpectedly jumps to an unexpected address. I.e.
branch shouldn't be taken at all, but even if it were it should have
been to 0x13e26e4 and not to 0x5.
Was about to write that the generated OP for brz,pn usually looks
different, when realized that in fact it was even generated for this
very address just before, but with another branch in the delay slot.
The bug looks familiar, Blue, isn't it? :)
>>>
>>> Sorry, does not ring a bell.
>>
>> I meant c27e275 where you fixed unconditional branch in a delay slot.
>> (One of my first bug reports).
>> Now it looks pretty similar for the conditional branches.
>>
IN:
0x013e26c0: brz,pn %o0, 0x13e26e4
0x013e26c4: brlez,pn %o1, 0x13e26e4
OP:
0x13e26c0
ld_i64 tmp6,regwptr,$0x0
movi_i64 cond,$0x0
movi_i64 tmp8,$0x0
brcond_i64 tmp6,tmp8,ne,$0x0
movi_i64 cond,$0x1
set_label $0x0
^^^ Ok, that's how brz,pn usually looks like
0x13e26c4
ld_i64 tmp7,regwptr,$0x8
movi_i64 tmp8,$0x0
brcond_i64 cond,tmp8,eq,$0x1
movi_i64 npc,$0x13e26e4
br $0x2
set_label $0x1
movi_i64 npc,$0x13e26c8
set_label $0x2
movi_i64 cond,$0x0
movi_i64 tmp8,$0x0
brcond_i64 tmp7,tmp8,gt,$0x3
movi_i64 cond,$0x1
set_label $0x3
movi_i64 tmp0,$0x0
brcond_i64 cond,tmp0,eq,$0x4
movi_i64 npc,$0x13e26e4
br $0x5
set_label $0x4
movi_i64 npc,$0x5
set_label $0x5
exit_tb $0x0
--
IN:
0x013e26c0: brz,pn %o0, 0x13e26e4
OP:
0x13e26c0
ld_i64 tmp6,regwptr,$0x0
movi_i64 cond,$0x0
movi_i64 tmp8,$0x0
brcond_i64 tmp6,tmp8,ne,$0x0
movi_i64 cond,$0x1
set_label $0x0
movi_i64 pc,$0x5
^^^ What's that?
>>>
>>> Probably DYNAMIC_PC + 4. I guess we are hitting this ancient comment
>>> in target-sparc/translate.c:1372:
>>> /* XXX: potentially incorrect if dynamic npc */
>>
>> Yes, I think this too. The following patch passes my tests. Do you
>> think it's correct? If yes, I'll make it for the other branches too.
>
> Looks OK. All these almost identical checks are a worrying: are all
> cases covered? Is the logic same when it should be? Perhaps there
> should be centralized handling, for example gen_next_pc_branch()
> gen_next_pc_delay_slot() etc. with asserts.
Sounds reasonable. Also do_branch and do_fbranch handle unconditional
cases absolutely identically. Could do something like
if (!do_unconditional_branch()) {
gen_fcond() // gen_cond()
...
>
> Also reusing dc->pc etc for in band signaling is not robust as this case
> shows.
>
>> @@ -1384,8 +1399,14 @@ static void do_branch_reg(DisasContext *dc,
>> int32_t offset, uint32_t insn,
>> } else {
>> dc->pc = dc->npc;
>> dc->jump_pc[0] = target;
>> - dc->jump_pc[1] = dc->npc + 4;
>> - dc->npc = JUMP_PC;
>> + if (unlikely(dc->npc == DYNAMIC_PC)) {
>> + dc->jump_pc[1] = DYNAMIC_PC;
>> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> +
>> + } else {
>> + dc->jump_pc[1] = dc->npc + 4;
>> + dc->npc = JUMP_PC;
>> + }
>>
>>
>> Regards,
>> Artyom Tarasenko
>>
>> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
[Qemu-devel] [PATCH][SPARC] Fix handling of conditional branches in delay slot of a conditional branch
Check whether dc->npc is dynamic before using its value for branch.
Signed-off-by: Artyom Tarasenko
---
Particaluary the patch fixes handling of the constructions like
0x13e26c0: brz,pn %o0, 0x13e26e4
0x13e26c4: brlez,pn %o1, 0x13e26e4
present in NetBSD-5.1
target-sparc/translate.c | 30 +-
1 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 958fbc5..dee67b3 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond,
TCGv r_src)
}
#endif
-/* XXX: potentially incorrect if dynamic npc */
static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
TCGv r_cond)
{
@@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t offset,
uint32_t insn, int cc,
} else {
dc->pc = dc->npc;
dc->jump_pc[0] = target;
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
+if (unlikely(dc->npc == DYNAMIC_PC)) {
+dc->jump_pc[1] = DYNAMIC_PC;
+tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+} else {
+dc->jump_pc[1] = dc->npc + 4;
+dc->npc = JUMP_PC;
+}
}
}
}
-/* XXX: potentially incorrect if dynamic npc */
static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int cc,
TCGv r_cond)
{
@@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t
offset, uint32_t insn, int cc,
} else {
dc->pc = dc->npc;
dc->jump_pc[0] = target;
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
+if (unlikely(dc->npc == DYNAMIC_PC)) {
+dc->jump_pc[1] = DYNAMIC_PC;
+tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+} else {
+dc->jump_pc[1] = dc->npc + 4;
+dc->npc = JUMP_PC;
+}
}
}
}
#ifdef TARGET_SPARC64
-/* XXX: potentially incorrect if dynamic npc */
static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
TCGv r_cond, TCGv r_reg)
{
@@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t
offset, uint32_t insn,
} else {
dc->pc = dc->npc;
dc->jump_pc[0] = target;
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
+if (unlikely(dc->npc == DYNAMIC_PC)) {
+dc->jump_pc[1] = DYNAMIC_PC;
+tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+} else {
+dc->jump_pc[1] = dc->npc + 4;
+dc->npc = JUMP_PC;
+}
}
}
--
1.7.3.4
Re: [Qemu-devel] [PATCH 1/2] ptimer: move declarations to ptimer.h
On 08/05/2011 05:56 PM, Anthony Liguori wrote: On 08/02/2011 06:47 AM, Paolo Bonzini wrote: Since the next patch will move VMState declarations for ptimers out of hw/hw.h, prepare a place for them. Signed-off-by: Paolo Bonzini --- hw/arm_timer.c | 1 + hw/etraxfs_timer.c | 1 + hw/grlib_apbuart.c | 1 + hw/grlib_gptimer.c | 1 + hw/lan9118.c | 1 + hw/leon3.c | 1 + hw/lm32_timer.c | 1 + hw/mcf5206.c | 1 + hw/mcf5208.c | 1 + hw/milkymist-sysctl.c | 1 + hw/musicpal.c | 1 + hw/ptimer.c | 1 + hw/ptimer.h | 27 +++ hw/sh_timer.c | 1 + hw/slavio_timer.c | 1 + With this series applied, I get: In file included from /home/anthony/git/qemu/hw/slavio_timer.c:27:0: /home/anthony/git/qemu/hw/ptimer.h:27:33: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘vmstate_ptimer’ /home/anthony/git/qemu/hw/slavio_timer.c:338:9: error: ‘vmstate_ptimer’ undeclared here (not in a function) make[1]: *** [slavio_timer.o] Error 1 make: *** [subdir-sparc-softmmu] Error 2 Never attribute to chance what can be attributed to stupidity. Forgot I had configured with --target-list. Paolo
Re: [Qemu-devel] [PATCH] TCG: fix copy propagation
Am 06.08.2011 16:06, schrieb Blue Swirl:
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 13 +++--
tcg/tcg.h | 5 +
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..748ecf9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,13 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
- int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+ TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
- if (src >= nb_globals) {
+ if (src >= nb_globals &&
+ tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
[snip]
Hi Blue,
your patch fixes qemu-system-x86_64 which now seems to work on 32 bit
hosts, too.
qemu-system-mips64(el) still fail with the same abort. They work when I
remove the
if block in tcg_opt_gen_mov.
The Debian kernel for qemu-system-mips64 which I used for the test is
available on
http://qemu.weilnetz.de/mips64/.
I could not reproduce the crash with qemu-system-ppc64 - neither with
nor without
your patch.
Kind regards,
Stefan
Re: [Qemu-devel] [PATCH][SPARC] Fix handling of conditional branches in delay slot of a conditional branch
Thanks, applied.
On Sat, Aug 6, 2011 at 3:01 PM, Artyom Tarasenko wrote:
> Check whether dc->npc is dynamic before using its value for branch.
>
> Signed-off-by: Artyom Tarasenko
> ---
> Particaluary the patch fixes handling of the constructions like
>
> 0x13e26c0: brz,pn %o0, 0x13e26e4
> 0x13e26c4: brlez,pn %o1, 0x13e26e4
>
> present in NetBSD-5.1
>
> target-sparc/translate.c | 30 +-
> 1 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 958fbc5..dee67b3 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond,
> TCGv r_src)
> }
> #endif
>
> -/* XXX: potentially incorrect if dynamic npc */
> static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int
> cc,
> TCGv r_cond)
> {
> @@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t
> offset, uint32_t insn, int cc,
> } else {
> dc->pc = dc->npc;
> dc->jump_pc[0] = target;
> - dc->jump_pc[1] = dc->npc + 4;
> - dc->npc = JUMP_PC;
> + if (unlikely(dc->npc == DYNAMIC_PC)) {
> + dc->jump_pc[1] = DYNAMIC_PC;
> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> + } else {
> + dc->jump_pc[1] = dc->npc + 4;
> + dc->npc = JUMP_PC;
> + }
> }
> }
> }
>
> -/* XXX: potentially incorrect if dynamic npc */
> static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int
> cc,
> TCGv r_cond)
> {
> @@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t
> offset, uint32_t insn, int cc,
> } else {
> dc->pc = dc->npc;
> dc->jump_pc[0] = target;
> - dc->jump_pc[1] = dc->npc + 4;
> - dc->npc = JUMP_PC;
> + if (unlikely(dc->npc == DYNAMIC_PC)) {
> + dc->jump_pc[1] = DYNAMIC_PC;
> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> + } else {
> + dc->jump_pc[1] = dc->npc + 4;
> + dc->npc = JUMP_PC;
> + }
> }
> }
> }
>
> #ifdef TARGET_SPARC64
> -/* XXX: potentially incorrect if dynamic npc */
> static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
> TCGv r_cond, TCGv r_reg)
> {
> @@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t
> offset, uint32_t insn,
> } else {
> dc->pc = dc->npc;
> dc->jump_pc[0] = target;
> - dc->jump_pc[1] = dc->npc + 4;
> - dc->npc = JUMP_PC;
> + if (unlikely(dc->npc == DYNAMIC_PC)) {
> + dc->jump_pc[1] = DYNAMIC_PC;
> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
> + } else {
> + dc->jump_pc[1] = dc->npc + 4;
> + dc->npc = JUMP_PC;
> + }
> }
> }
>
> --
> 1.7.3.4
>
>
Re: [Qemu-devel] [PATCH] TCG: fix copy propagation
Am 06.08.2011 22:13, schrieb Stefan Weil:
Am 06.08.2011 16:06, schrieb Blue Swirl:
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 13 +++--
tcg/tcg.h | 5 +
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..748ecf9 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,13 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
- int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg
dst,
+ TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
- if (src >= nb_globals) {
+ if (src >= nb_globals &&
+ tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
[snip]
Hi Blue,
your patch fixes qemu-system-x86_64 which now seems to work on 32 bit
hosts, too.
qemu-system-mips64(el) still fail with the same abort. They work when
I remove the
if block in tcg_opt_gen_mov.
The Debian kernel for qemu-system-mips64 which I used for the test is
available on
http://qemu.weilnetz.de/mips64/.
I could not reproduce the crash with qemu-system-ppc64 - neither with
nor without
your patch.
Kind regards,
Stefan
The patch works with qemu-system-mips64(el) after a small modification:
if (src >= nb_globals &&
tcg_arg_is_local(s, src) && tcg_arg_is_local(s, dst)) {
Cheers,
Stefan
[Qemu-devel] [PATCH] configure: Disable guest_agent for mingw32
guest_agent is not supported for mingw32, so the default value
should be 'no', not 'yes'.
This removes the dependencies to glib-2.0 and python which
makes native and cross builds for w32 much easier (no need
to get and install these extra packages).
It also avoids the problems caused by different bitfield alignment
which is required by glib-2.0.
It is still possible to set guest_agent=yes via configure option.
Signed-off-by: Stefan Weil
---
configure |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 0c67a4a..4cb33d9 100755
--- a/configure
+++ b/configure
@@ -493,6 +493,7 @@ if test "$mingw32" = "yes" ; then
bindir="\${prefix}"
sysconfdir="\${prefix}"
confsuffix=""
+ guest_agent="no"
fi
werror=""
--
1.7.2.5
Re: [Qemu-devel] [PATCH] TCG: fix copy propagation
On Sat, Aug 6, 2011 at 8:33 PM, Stefan Weil wrote:
> Am 06.08.2011 22:13, schrieb Stefan Weil:
>>
>> Am 06.08.2011 16:06, schrieb Blue Swirl:
>>>
>>> Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
>>> considered only global registers. However, register temps and stack
>>> allocated locals must be handled differently because register temps
>>> don't survive across brcond.
>>>
>>> Fix by propagating only within same class of temps.
>>>
>>> Signed-off-by: Blue Swirl
>>> ---
>>> tcg/optimize.c | 13 +++--
>>> tcg/tcg.h | 5 +
>>> 2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tcg/optimize.c b/tcg/optimize.c
>>> index a3bfa5e..748ecf9 100644
>>> --- a/tcg/optimize.c
>>> +++ b/tcg/optimize.c
>>> @@ -185,12 +185,13 @@ static int op_to_movi(int op)
>>> }
>>> }
>>>
>>> -static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
>>> - int nb_temps, int nb_globals)
>>> +static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
>>> + TCGArg src, int nb_temps, int nb_globals)
>>> {
>>> reset_temp(dst, nb_temps, nb_globals);
>>> assert(temps[src].state != TCG_TEMP_COPY);
>>> - if (src >= nb_globals) {
>>> + if (src >= nb_globals &&
>>> + tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
>>> assert(temps[src].state != TCG_TEMP_CONST);
>>
>> [snip]
>>
>> Hi Blue,
>>
>> your patch fixes qemu-system-x86_64 which now seems to work on 32 bit
>> hosts, too.
>>
>> qemu-system-mips64(el) still fail with the same abort. They work when I
>> remove the
>> if block in tcg_opt_gen_mov.
>>
>> The Debian kernel for qemu-system-mips64 which I used for the test is
>> available on
>> http://qemu.weilnetz.de/mips64/.
>>
>> I could not reproduce the crash with qemu-system-ppc64 - neither with nor
>> without
>> your patch.
>>
>> Kind regards,
>> Stefan
>
> The patch works with qemu-system-mips64(el) after a small modification:
>
> if (src >= nb_globals &&
> tcg_arg_is_local(s, src) && tcg_arg_is_local(s, dst)) {
This removes the optimization completely for register temps. I think
there is a better solution.
The problem is similar to x86_64:
IN: free_area_init_nodes
0x806004d8: sw v0,21664(s5)
0x806004dc: lw v1,0(s0)
0x806004e0: luiv0,0x8062
0x806004e4: sw v1,21676(v0)
0x806004e8: sw v1,4(s8)
0x806004ec: lw a0,4(s0)
0x806004f0: move a1,zero
0x806004f4: sltu v0,a0,v1
0x806004f8: movz v1,a0,v0
0x806004fc: luiv0,0x8062
0x80600500: addiu s4,v0,21676
0x80600504: luiv0,0x8062
0x80600508: sw v1,4(s4)
0x8060050c: addiu a0,v0,21688
0x80600510: li a2,4
0x80600514: sw zero,8(s8)
0x80600518: jal0x80104000
0x8060051c: sw zero,8(s4)
OP:
0x806004d8
movi_i32 tmp0,$0x54a0
movi_i32 tmp1,$0x0
add2_i32 tmp0,tmp1,s5_0,s5_1,tmp0,tmp1
mov_i32 tmp2,v0_0
mov_i32 tmp3,v0_1
qemu_st32 tmp2,tmp0,tmp1,$0x0
0x806004dc
mov_i32 tmp2,s0_0
mov_i32 tmp3,s0_1
qemu_ld32 tmp2,tmp2,tmp3,$0x0
movi_i32 tmp4,$0x1f
sar_i32 tmp3,tmp2,tmp4
mov_i32 v1_0,tmp2
mov_i32 v1_1,tmp3
0x806004e0
movi_i32 v0_0,$0x8062
movi_i32 v0_1,$0x
0x806004e4
movi_i32 tmp0,$0x54ac
movi_i32 tmp1,$0x0
add2_i32 tmp0,tmp1,v0_0,v0_1,tmp0,tmp1
mov_i32 tmp2,v1_0
mov_i32 tmp3,v1_1
qemu_st32 tmp2,tmp0,tmp1,$0x0
0x806004e8
movi_i32 tmp2,$0x4
movi_i32 tmp3,$0x0
add2_i32 tmp2,tmp3,s8_0,s8_1,tmp2,tmp3
mov_i32 tmp0,v1_0
mov_i32 tmp1,v1_1
qemu_st32 tmp0,tmp2,tmp3,$0x0
0x806004ec
movi_i32 tmp0,$0x4
movi_i32 tmp1,$0x0
add2_i32 tmp0,tmp1,s0_0,s0_1,tmp0,tmp1
qemu_ld32 tmp0,tmp0,tmp1,$0x0
movi_i32 tmp4,$0x1f
sar_i32 tmp1,tmp0,tmp4
mov_i32 a0_0,tmp0
mov_i32 a0_1,tmp1
0x806004f0
movi_i32 a1_0,$0x0
movi_i32 a1_1,$0x0
0x806004f4
mov_i32 tmp2,a0_0
mov_i32 tmp3,a0_1
mov_i32 tmp0,v1_0
mov_i32 tmp1,v1_1
setcond2_i32 v0_0,tmp2,tmp3,tmp0,tmp1,ltu
movi_i32 v0_1,$0x0
0x806004f8
movi_i32 tmp0,$0x0
movi_i32 tmp1,$0x0
brcond2_i32 v0_0,v0_1,tmp0,tmp1,ne,$0x0
mov_i32 v1_0,a0_0
mov_i32 v1_1,a0_1
set_label $0x0
Here a0 locals are used after brcond.
0x806004fc
movi_i32 v0_0,$0x8062
movi_i32 v0_1,$0x
0x80600500
movi_i32 tmp0,$0x54ac
movi_i32 tmp1,$0x0
add2_i32 s4_0,s4_1,v0_0,v0_1,tmp0,tmp1
movi_i32 tmp4,$0x1f
sar_i32 s4_1,s4_0,tmp4
0x80600504
movi_i32 v0_0,$0x8062
movi_i32 v0_1,$0x
0x80600508
movi_i32 tmp0,$0x4
movi_i32 tmp1,$0x0
add2_i32 tmp0,tmp1,s4_0,s4_1,tmp0,tmp1
mov_i32 tmp2,v1_0
mov_i32 tmp3,v1_1
qemu_st32 tmp2,tmp0,tmp1,$0x0
0x8060050c
movi_i32 tmp2,$0x54b8
movi_i32 tmp3,$0x0
add2_i32 a0_0,a0_1,v0_0,v0_1,tmp2,tmp3
movi_i32 tmp4,$0x1f
sar_i32 a0_1,a0_0,tmp4
0x80600510
movi_i32 a2_0,$0x4
movi_i32
[Qemu-devel] [PATCH] TCG: fix copy propagation
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 15 +--
tcg/tcg.h |5 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..7eb5eb1 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,15 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
-int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
-if (src >= nb_globals) {
+/* Don't try to copy if one of temps is a global or either one
+ is local and another is register */
+if (src >= nb_globals && dst >= nb_globals &&
+tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
if (temps[src].state != TCG_TEMP_HAS_COPY) {
temps[src].state = TCG_TEMP_HAS_COPY;
@@ -474,7 +477,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 3;
@@ -500,7 +503,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1], nb_temps,
+tcg_opt_gen_mov(s, gen_args, args[0], args[1], nb_temps,
nb_globals);
gen_args += 2;
args += 3;
@@ -523,7 +526,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
break;
}
if (temps[args[1]].state != TCG_TEMP_CONST) {
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 2;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index e76f9af..e2a7095 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -410,6 +410,11 @@ static inline TCGv_i64 tcg_temp_local_new_i64(void)
void tcg_temp_free_i64(TCGv_i64 arg);
char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size,
TCGv_i64 arg);
+static inline bool tcg_arg_is_local(TCGContext *s, TCGArg arg)
+{
+return s->temps[arg].temp_local;
+}
+
#if defined(CONFIG_DEBUG_TCG)
/* If you call tcg_clear_temp_count() at the start of a section of
* code which is not supposed to leak any TCG temporaries, then
--
1.6.2.4
From c06136b49409ba1c68457ce3ceffb5eeb355ec47 Mon Sep 17 00:00:00 2001
Message-Id:
From: Blue Swirl
Date: Sat, 6 Aug 2011 13:58:47 +
Subject: [PATCH] TCG: fix copy propagation
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 15 +--
tcg/tcg.h |5 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..7eb5eb1 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,15 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
-int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
-if (src >= nb_globals) {
+/* Don't try to copy if one of temps is a global or either one
+ is local and another is register */
+if (src >= nb_globals && dst >= nb_globals &&
+tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
Re: [Qemu-devel] [PATCH][SPARC] Fix handling of conditional branches in delay slot of a conditional branch
Since it's a pure bug fix, do you think can it be applied to 0.15 as well?
On Sat, Aug 6, 2011 at 10:14 PM, Blue Swirl wrote:
> Thanks, applied.
>
> On Sat, Aug 6, 2011 at 3:01 PM, Artyom Tarasenko wrote:
>> Check whether dc->npc is dynamic before using its value for branch.
>>
>> Signed-off-by: Artyom Tarasenko
>> ---
>> Particaluary the patch fixes handling of the constructions like
>>
>> 0x13e26c0: brz,pn %o0, 0x13e26e4
>> 0x13e26c4: brlez,pn %o1, 0x13e26e4
>>
>> present in NetBSD-5.1
>>
>> target-sparc/translate.c | 30 +-
>> 1 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>> index 958fbc5..dee67b3 100644
>> --- a/target-sparc/translate.c
>> +++ b/target-sparc/translate.c
>> @@ -1286,7 +1286,6 @@ static inline void gen_cond_reg(TCGv r_dst, int cond,
>> TCGv r_src)
>> }
>> #endif
>>
>> -/* XXX: potentially incorrect if dynamic npc */
>> static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int
>> cc,
>> TCGv r_cond)
>> {
>> @@ -1321,13 +1320,17 @@ static void do_branch(DisasContext *dc, int32_t
>> offset, uint32_t insn, int cc,
>> } else {
>> dc->pc = dc->npc;
>> dc->jump_pc[0] = target;
>> - dc->jump_pc[1] = dc->npc + 4;
>> - dc->npc = JUMP_PC;
>> + if (unlikely(dc->npc == DYNAMIC_PC)) {
>> + dc->jump_pc[1] = DYNAMIC_PC;
>> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> + } else {
>> + dc->jump_pc[1] = dc->npc + 4;
>> + dc->npc = JUMP_PC;
>> + }
>> }
>> }
>> }
>>
>> -/* XXX: potentially incorrect if dynamic npc */
>> static void do_fbranch(DisasContext *dc, int32_t offset, uint32_t insn, int
>> cc,
>> TCGv r_cond)
>> {
>> @@ -1362,14 +1365,18 @@ static void do_fbranch(DisasContext *dc, int32_t
>> offset, uint32_t insn, int cc,
>> } else {
>> dc->pc = dc->npc;
>> dc->jump_pc[0] = target;
>> - dc->jump_pc[1] = dc->npc + 4;
>> - dc->npc = JUMP_PC;
>> + if (unlikely(dc->npc == DYNAMIC_PC)) {
>> + dc->jump_pc[1] = DYNAMIC_PC;
>> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> + } else {
>> + dc->jump_pc[1] = dc->npc + 4;
>> + dc->npc = JUMP_PC;
>> + }
>> }
>> }
>> }
>>
>> #ifdef TARGET_SPARC64
>> -/* XXX: potentially incorrect if dynamic npc */
>> static void do_branch_reg(DisasContext *dc, int32_t offset, uint32_t insn,
>> TCGv r_cond, TCGv r_reg)
>> {
>> @@ -1384,8 +1391,13 @@ static void do_branch_reg(DisasContext *dc, int32_t
>> offset, uint32_t insn,
>> } else {
>> dc->pc = dc->npc;
>> dc->jump_pc[0] = target;
>> - dc->jump_pc[1] = dc->npc + 4;
>> - dc->npc = JUMP_PC;
>> + if (unlikely(dc->npc == DYNAMIC_PC)) {
>> + dc->jump_pc[1] = DYNAMIC_PC;
>> + tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
>> + } else {
>> + dc->jump_pc[1] = dc->npc + 4;
>> + dc->npc = JUMP_PC;
>> + }
>> }
>> }
>>
>> --
>> 1.7.3.4
>>
>>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
[Qemu-devel] Votre site Web pour 999 DT seulement !
PROMOTION SITE WEB Du 01 au 20 Août 2011 Nous proposons aux entreprises/associations une solution Internet clé en main à 999 Dinars. Grande ou petite, votre entreprise/association ne peut plus se passer d’un site internet si elle désire grandir et se faire connaitre du plus grand nombre. Nous réalisons pour vous un site web moderne et efficace, entièrement et facilement gérable par vous-même au meilleur rapport qualité prix. L'offre comprend: • 10 pages web. • Back Office sécurisé : gestion du contenu du site. • Formulaire de contact • Une conception graphique moderne • Module de news • inscription newsletter • Moteur de recherche interne • Galerie photo • liens vers les réseaux sociaux (facebook, twitter...) • Module de sondage • Statistiques de fréquentation • Images et textes fournis par vos soins Commander maintenant : http://ptf.tweensa.com/link.php?M=4129515&N=122&L=37&F=T
[Qemu-devel] buildbot failure in qemu on xen_i386_debian_5_0
The Buildbot has detected a new failure on builder xen_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_i386_debian_5_0/builds/57 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on xen_x86_64_debian_5_0
The Buildbot has detected a new failure on builder xen_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_x86_64_debian_5_0/builds/57 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
Re: [Qemu-devel] [PATCH] TCG: fix copy propagation
Am 06.08.2011 23:26, schrieb Blue Swirl:
Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
considered only global registers. However, register temps and stack
allocated locals must be handled differently because register temps
don't survive across brcond.
Fix by propagating only within same class of temps.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 15 +--
tcg/tcg.h |5 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..7eb5eb1 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -185,12 +185,15 @@ static int op_to_movi(int op)
}
}
-static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
-int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
+TCGArg src, int nb_temps, int nb_globals)
{
reset_temp(dst, nb_temps, nb_globals);
assert(temps[src].state != TCG_TEMP_COPY);
-if (src>= nb_globals) {
+/* Don't try to copy if one of temps is a global or either one
+ is local and another is register */
+if (src>= nb_globals&& dst>= nb_globals&&
+tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
assert(temps[src].state != TCG_TEMP_CONST);
if (temps[src].state != TCG_TEMP_HAS_COPY) {
temps[src].state = TCG_TEMP_HAS_COPY;
@@ -474,7 +477,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 3;
@@ -500,7 +503,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
gen_opc_buf[op_index] = INDEX_op_nop;
} else {
gen_opc_buf[op_index] = op_to_mov(op);
-tcg_opt_gen_mov(gen_args, args[0], args[1], nb_temps,
+tcg_opt_gen_mov(s, gen_args, args[0], args[1], nb_temps,
nb_globals);
gen_args += 2;
args += 3;
@@ -523,7 +526,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
break;
}
if (temps[args[1]].state != TCG_TEMP_CONST) {
-tcg_opt_gen_mov(gen_args, args[0], args[1],
+tcg_opt_gen_mov(s, gen_args, args[0], args[1],
nb_temps, nb_globals);
gen_args += 2;
args += 2;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index e76f9af..e2a7095 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -410,6 +410,11 @@ static inline TCGv_i64 tcg_temp_local_new_i64(void)
void tcg_temp_free_i64(TCGv_i64 arg);
char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size,
TCGv_i64 arg);
+static inline bool tcg_arg_is_local(TCGContext *s, TCGArg arg)
+{
+return s->temps[arg].temp_local;
+}
+
#if defined(CONFIG_DEBUG_TCG)
/* If you call tcg_clear_temp_count() at the start of a section of
* code which is not supposed to leak any TCG temporaries, then
This fixes qemu-system-x86_64 and qemu-system-mips64(el) on 32 bit hosts.
Tested-by: Stefan Weil
