[Qemu-devel] [PATCH] Switch the roles of jmp_next[0] and jmp_next[1].

[Qemu-devel] [PATCH] Switch the roles of jmp_next[0] and jmp_next[1].

[Zhihui Zhang]

This way, jmp_next[0] means "taken" for both conditional jumps AND unconditional jumps.
Currently, it has opposite meanings for the two cases.

Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>
---
 target-i386/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 14b0298..7daa1a1 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2430,10 +2430,10 @@ static inline void gen_jcc(DisasContext *s, int b,
         l1 = gen_new_label();
         gen_jcc1(s, b, l1);
 
-        gen_goto_tb(s, 0, next_eip);
+        gen_goto_tb(s, 1, next_eip);
 
         gen_set_label(l1);
-        gen_goto_tb(s, 1, val);
+        gen_goto_tb(s, 0, val);
         s->is_jmp = DISAS_TB_JUMP;
     } else {
         l1 = gen_new_label();
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH] Switch the roles of jmp_next[0] and jmp_next[1].

[Peter Maydell]

On 22 June 2013 17:10, Zhihui Zhang <zzhsuny@gmail.com> wrote:
> This way, jmp_next[0] means "taken" for both conditional jumps AND unconditional jumps.
> Currently, it has opposite meanings for the two cases.

This commit message is missing a rationale: do you want
to do this because:
 * it fixes a bug? (if so, what bug)
 * it is more efficient? (if so, some description of why)
 * you just think it would be neater this way around?

How about the TB exit via gen_jz_ecx_string ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Switch the roles of jmp_next[0] and jmp_next[1].

[Richard Henderson]

On 06/22/2013 09:10 AM, Zhihui Zhang wrote:
> This way, jmp_next[0] means "taken" for both conditional jumps AND unconditional jumps.
> Currently, it has opposite meanings for the two cases.

So?  There's no proscribed meaning for the 0 and 1 indices.


r~

Re: [Qemu-devel] [PATCH] Switch the roles of jmp_next[0] and jmp_next[1].

[Zhihui Zhang]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

My intention was to make things consistent for readability.  In the case of
gen_jz_ecx_string(), jmp_next[1] is already the "not taken" case.  However,
looks like the meaning of jmp_next changes in the case of a TB loop, it
does not point to the next TB to execute any more. Anyway, I thought this
patch can reduce some confusion.


On Sun, Jun 23, 2013 at 2:22 PM, Richard Henderson <rth@twiddle.net> wrote:

> On 06/22/2013 09:10 AM, Zhihui Zhang wrote:
> > This way, jmp_next[0] means "taken" for both conditional jumps AND
> unconditional jumps.
> > Currently, it has opposite meanings for the two cases.
>
> So?  There's no proscribed meaning for the 0 and 1 indices.
>
>
> r~
>

[-- Attachment #2: Type: text/html, Size: 1117 bytes --]