* [Qemu-devel] [PATCH] i386: fix icount processing for repz instructions
@ 2014-10-21 13:03 Pavel Dovgalyuk
2014-10-23 17:05 ` Richard Henderson
0 siblings, 1 reply; 3+ messages in thread
From: Pavel Dovgalyuk @ 2014-10-21 13:03 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, batuzovk, maria.klimushenkova, pavel.dovgaluk,
pbonzini, zealot351, fred.konrad
TCG generates optimized code for i386 repz instructions. 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.
Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
target-i386/translate.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 418173e..1284173 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,18 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu,
|| (flags & HF_SOFTMMU_MASK)
#endif
);
+ dc->repz_opt = dc->jmp_opt
+ /* 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
+ trace and replay modes and there will always be an
+ additional step for ecx=0.
+ */
+ || use_icount;
#if 0
/* check addseg logic */
if (!dc->addseg && (dc->vm86 || !dc->pe || !dc->code32))
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] i386: fix icount processing for repz instructions
2014-10-21 13:03 [Qemu-devel] [PATCH] i386: fix icount processing for repz instructions Pavel Dovgalyuk
@ 2014-10-23 17:05 ` Richard Henderson
2014-10-23 20:19 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Richard Henderson @ 2014-10-23 17:05 UTC (permalink / raw)
To: Pavel Dovgalyuk, qemu-devel
Cc: mark.burton, batuzovk, maria.klimushenkova, pbonzini, zealot351,
fred.konrad
On 10/21/2014 06:03 AM, Pavel Dovgalyuk wrote:
> + dc->repz_opt = dc->jmp_opt
> + /* 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
> + trace and replay modes and there will always be an
> + additional step for ecx=0.
> + */
> + || use_icount;
My aesthetics are offended by the placement of this comment. Please write
/* Comment */
dc->repz_opt = x || y;
That said, surely that test should be !use_icount.
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] i386: fix icount processing for repz instructions
2014-10-23 17:05 ` Richard Henderson
@ 2014-10-23 20:19 ` Paolo Bonzini
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2014-10-23 20:19 UTC (permalink / raw)
To: Richard Henderson, Pavel Dovgalyuk, qemu-devel
Cc: fred.konrad, zealot351, maria.klimushenkova, mark.burton,
batuzovk
On 10/23/2014 07:05 PM, Richard Henderson wrote:
> On 10/21/2014 06:03 AM, Pavel Dovgalyuk wrote:
>> + dc->repz_opt = dc->jmp_opt
>> + /* 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
>> + trace and replay modes and there will always be an
>> + additional step for ecx=0.
>> + */
>> + || use_icount;
>
> My aesthetics are offended by the placement of this comment. Please write
>
> /* Comment */
> dc->repz_opt = x || y;
>
> That said, surely that test should be !use_icount.
And "&& !use_icount", even (it becomes "|| use_icount" in the ifs)?
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-10-23 20:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-21 13:03 [Qemu-devel] [PATCH] i386: fix icount processing for repz instructions Pavel Dovgalyuk
2014-10-23 17:05 ` Richard Henderson
2014-10-23 20:19 ` Paolo Bonzini
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).