qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <rth@twiddle.net>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Subject: [Qemu-devel] [PULL 3/3] target-i386: fix icount processing for repz instructions
Date: Sun, 14 Dec 2014 17:00:48 -0600	[thread overview]
Message-ID: <1418598048-21995-4-git-send-email-rth@twiddle.net> (raw)
In-Reply-To: <1418598048-21995-1-git-send-email-rth@twiddle.net>

From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

TCG generates optimized code for i386 repz instructions in single step mode.
It means that when ecx becomes 0, execution of the string instruction breaks
immediately without an additional iteration for ecx==0 (which will only check
ecx and set the flags). Omitting this iteration leads to different
instructions counting in singlestep mode and in normal execution.
This patch disables optimization of this last iteration for icount mode
which should be deterministic.

v2: inverted the condition and formatted the comment

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 782f7d2..6243e36 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -115,6 +115,7 @@ typedef struct DisasContext {
     int tf;     /* TF cpu flag */
     int singlestep_enabled; /* "hardware" single step enabled */
     int jmp_opt; /* use direct block chaining for direct jumps */
+    int repz_opt; /* optimize jumps within repz instructions */
     int mem_index; /* select memory access functions */
     uint64_t flags; /* all execution flags */
     struct TranslationBlock *tb;
@@ -1215,7 +1216,7 @@ static inline void gen_repz_ ## op(DisasContext *s, TCGMemOp ot,              \
     gen_op_add_reg_im(s->aflag, R_ECX, -1);                                   \
     /* a loop would cause two single step exceptions if ECX = 1               \
        before rep string_insn */                                              \
-    if (!s->jmp_opt)                                                          \
+    if (s->repz_opt)                                                          \
         gen_op_jz_ecx(s->aflag, l2);                                          \
     gen_jmp(s, cur_eip);                                                      \
 }
@@ -1233,7 +1234,7 @@ static inline void gen_repz_ ## op(DisasContext *s, TCGMemOp ot,              \
     gen_op_add_reg_im(s->aflag, R_ECX, -1);                                   \
     gen_update_cc_op(s);                                                      \
     gen_jcc1(s, (JCC_Z << 1) | (nz ^ 1), l2);                                 \
-    if (!s->jmp_opt)                                                          \
+    if (s->repz_opt)                                                          \
         gen_op_jz_ecx(s->aflag, l2);                                          \
     gen_jmp(s, cur_eip);                                                      \
 }
@@ -7951,6 +7952,17 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu,
                     || (flags & HF_SOFTMMU_MASK)
 #endif
                     );
+    /* Do not optimize repz jumps at all in icount mode, because
+       rep movsS instructions are execured with different paths
+       in !repz_opt and repz_opt modes. The first one was used
+       always except single step mode. And this setting
+       disables jumps optimization and control paths become
+       equivalent in run and single step modes.
+       Now there will be no jump optimization for repz in
+       record/replay modes and there will always be an
+       additional step for ecx=0 when icount is enabled.
+     */
+    dc->repz_opt = !dc->jmp_opt && !use_icount;
 #if 0
     /* check addseg logic */
     if (!dc->addseg && (dc->vm86 || !dc->pe || !dc->code32))
-- 
2.1.0

  parent reply	other threads:[~2014-12-14 23:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-14 23:00 [Qemu-devel] [PULL 0/3] Collected target-i386 patches Richard Henderson
2014-12-14 23:00 ` [Qemu-devel] [PULL 1/3] target-i386: Wrong conversion infinity from float80 to int32/int64 Richard Henderson
2014-12-14 23:00 ` [Qemu-devel] [PULL 2/3] target-i386: fbld instruction doesn't set minus sign Richard Henderson
2014-12-14 23:00 ` Richard Henderson [this message]
2014-12-15 12:17 ` [Qemu-devel] [PULL 0/3] Collected target-i386 patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1418598048-21995-4-git-send-email-rth@twiddle.net \
    --to=rth@twiddle.net \
    --cc=pavel.dovgaluk@ispras.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).