* [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
@ 2013-04-10 3:30 liguang
2013-04-10 7:44 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: liguang @ 2013-04-10 3:30 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, proljc, e.voevodin, liguang, blauwirbel, paul,
pbonzini, afaerber, aurelien, rth
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
target-arm/translate.c | 17 ++++++++---------
target-i386/translate.c | 17 ++++++++---------
target-mips/translate.c | 16 ++++++++--------
3 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 35a21be..c0c080d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9806,11 +9806,10 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
cpu_M0 = tcg_temp_new_i64();
next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
lj = -1;
- num_insns = 0;
max_insns = tb->cflags & CF_COUNT_MASK;
- if (max_insns == 0)
+ if (max_insns == 0) {
max_insns = CF_COUNT_MASK;
-
+ }
gen_tb_start();
tcg_clear_temp_count();
@@ -9889,9 +9888,9 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
if (lj < j) {
- lj++;
- while (lj < j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj < j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
}
tcg_ctx.gen_opc_pc[lj] = dc->pc;
gen_opc_condexec_bits[lj] = (dc->condexec_cond << 4) | (dc->condexec_mask >> 1);
@@ -10028,9 +10027,9 @@ done_generating:
#endif
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
- lj++;
- while (lj <= j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj <= j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
} else {
tb->size = dc->pc - pc_start;
tb->icount = num_insns;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7596a90..9c5e1a3 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8319,11 +8319,10 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
dc->is_jmp = DISAS_NEXT;
pc_ptr = pc_start;
lj = -1;
- num_insns = 0;
max_insns = tb->cflags & CF_COUNT_MASK;
- if (max_insns == 0)
+ if (max_insns == 0) {
max_insns = CF_COUNT_MASK;
-
+ }
gen_tb_start();
for(;;) {
if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) {
@@ -8338,9 +8337,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
if (lj < j) {
- lj++;
- while (lj < j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj < j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
}
tcg_ctx.gen_opc_pc[lj] = pc_ptr;
gen_opc_cc_op[lj] = dc->cc_op;
@@ -8387,9 +8386,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
/* we don't forget to fill the last values */
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
- lj++;
- while (lj <= j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj <= j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
}
#ifdef DEBUG_DISAS
diff --git a/target-mips/translate.c b/target-mips/translate.c
index b7f8203..d1e5d84 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -15571,10 +15571,10 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
#else
ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
#endif
- num_insns = 0;
max_insns = tb->cflags & CF_COUNT_MASK;
- if (max_insns == 0)
+ if (max_insns == 0) {
max_insns = CF_COUNT_MASK;
+ }
LOG_DISAS("\ntb %p idx %d hflags %04x\n", tb, ctx.mem_idx, ctx.hflags);
gen_tb_start();
while (ctx.bstate == BS_NONE) {
@@ -15595,9 +15595,9 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
if (lj < j) {
- lj++;
- while (lj < j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj < j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
}
tcg_ctx.gen_opc_pc[lj] = ctx.pc;
gen_opc_hflags[lj] = ctx.hflags & MIPS_HFLAG_BMASK;
@@ -15678,9 +15678,9 @@ done_generating:
*tcg_ctx.gen_opc_ptr = INDEX_op_end;
if (search_pc) {
j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
- lj++;
- while (lj <= j)
- tcg_ctx.gen_opc_instr_start[lj++] = 0;
+ while (++lj <= j) {
+ tcg_ctx.gen_opc_instr_start[lj] = 0;
+ }
} else {
tb->size = ctx.pc - pc_start;
tb->icount = num_insns;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-10 3:30 [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal liguang
@ 2013-04-10 7:44 ` Paolo Bonzini
2013-04-10 7:55 ` li guang
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2013-04-10 7:44 UTC (permalink / raw)
To: liguang
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
Il 10/04/2013 05:30, liguang ha scritto:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
> target-arm/translate.c | 17 ++++++++---------
> target-i386/translate.c | 17 ++++++++---------
> target-mips/translate.c | 16 ++++++++--------
> 3 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 35a21be..c0c080d 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9806,11 +9806,10 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
> cpu_M0 = tcg_temp_new_i64();
> next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> lj = -1;
> - num_insns = 0;
Nack. Did you even try to read what I and Wei-Ren Chen were trying to
tell you?
Paolo
> max_insns = tb->cflags & CF_COUNT_MASK;
> - if (max_insns == 0)
> + if (max_insns == 0) {
> max_insns = CF_COUNT_MASK;
> -
> + }
> gen_tb_start();
>
> tcg_clear_temp_count();
> @@ -9889,9 +9888,9 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> if (lj < j) {
> - lj++;
> - while (lj < j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj < j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> }
> tcg_ctx.gen_opc_pc[lj] = dc->pc;
> gen_opc_condexec_bits[lj] = (dc->condexec_cond << 4) | (dc->condexec_mask >> 1);
> @@ -10028,9 +10027,9 @@ done_generating:
> #endif
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> - lj++;
> - while (lj <= j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj <= j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> } else {
> tb->size = dc->pc - pc_start;
> tb->icount = num_insns;
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 7596a90..9c5e1a3 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -8319,11 +8319,10 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> dc->is_jmp = DISAS_NEXT;
> pc_ptr = pc_start;
> lj = -1;
> - num_insns = 0;
> max_insns = tb->cflags & CF_COUNT_MASK;
> - if (max_insns == 0)
> + if (max_insns == 0) {
> max_insns = CF_COUNT_MASK;
> -
> + }
> gen_tb_start();
> for(;;) {
> if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) {
> @@ -8338,9 +8337,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> if (lj < j) {
> - lj++;
> - while (lj < j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj < j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> }
> tcg_ctx.gen_opc_pc[lj] = pc_ptr;
> gen_opc_cc_op[lj] = dc->cc_op;
> @@ -8387,9 +8386,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> /* we don't forget to fill the last values */
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> - lj++;
> - while (lj <= j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj <= j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> }
>
> #ifdef DEBUG_DISAS
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index b7f8203..d1e5d84 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -15571,10 +15571,10 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
> #else
> ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
> #endif
> - num_insns = 0;
> max_insns = tb->cflags & CF_COUNT_MASK;
> - if (max_insns == 0)
> + if (max_insns == 0) {
> max_insns = CF_COUNT_MASK;
> + }
> LOG_DISAS("\ntb %p idx %d hflags %04x\n", tb, ctx.mem_idx, ctx.hflags);
> gen_tb_start();
> while (ctx.bstate == BS_NONE) {
> @@ -15595,9 +15595,9 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> if (lj < j) {
> - lj++;
> - while (lj < j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj < j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> }
> tcg_ctx.gen_opc_pc[lj] = ctx.pc;
> gen_opc_hflags[lj] = ctx.hflags & MIPS_HFLAG_BMASK;
> @@ -15678,9 +15678,9 @@ done_generating:
> *tcg_ctx.gen_opc_ptr = INDEX_op_end;
> if (search_pc) {
> j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> - lj++;
> - while (lj <= j)
> - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> + while (++lj <= j) {
> + tcg_ctx.gen_opc_instr_start[lj] = 0;
> + }
> } else {
> tb->size = ctx.pc - pc_start;
> tb->icount = num_insns;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-10 7:44 ` Paolo Bonzini
@ 2013-04-10 7:55 ` li guang
2013-04-10 9:54 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: li guang @ 2013-04-10 7:55 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
在 2013-04-10三的 09:44 +0200，Paolo Bonzini写道：
> Il 10/04/2013 05:30, liguang ha scritto:
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> > target-arm/translate.c | 17 ++++++++---------
> > target-i386/translate.c | 17 ++++++++---------
> > target-mips/translate.c | 16 ++++++++--------
> > 3 files changed, 24 insertions(+), 26 deletions(-)
> >
> > diff --git a/target-arm/translate.c b/target-arm/translate.c
> > index 35a21be..c0c080d 100644
> > --- a/target-arm/translate.c
> > +++ b/target-arm/translate.c
> > @@ -9806,11 +9806,10 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
> > cpu_M0 = tcg_temp_new_i64();
> > next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> > lj = -1;
> > - num_insns = 0;
>
> Nack. Did you even try to read what I and Wei-Ren Chen were trying to
> tell you?
>
well, you ask if I tested, and I answer yes,
you doubt if this line really could be removed,
and I said compiler will not complain this.
what should I do?
Isn't it enough?
did you read my answer for your comment?
>
> > max_insns = tb->cflags & CF_COUNT_MASK;
> > - if (max_insns == 0)
> > + if (max_insns == 0) {
> > max_insns = CF_COUNT_MASK;
> > -
> > + }
> > gen_tb_start();
> >
> > tcg_clear_temp_count();
> > @@ -9889,9 +9888,9 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > if (lj < j) {
> > - lj++;
> > - while (lj < j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj < j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > }
> > tcg_ctx.gen_opc_pc[lj] = dc->pc;
> > gen_opc_condexec_bits[lj] = (dc->condexec_cond << 4) | (dc->condexec_mask >> 1);
> > @@ -10028,9 +10027,9 @@ done_generating:
> > #endif
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > - lj++;
> > - while (lj <= j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj <= j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > } else {
> > tb->size = dc->pc - pc_start;
> > tb->icount = num_insns;
> > diff --git a/target-i386/translate.c b/target-i386/translate.c
> > index 7596a90..9c5e1a3 100644
> > --- a/target-i386/translate.c
> > +++ b/target-i386/translate.c
> > @@ -8319,11 +8319,10 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> > dc->is_jmp = DISAS_NEXT;
> > pc_ptr = pc_start;
> > lj = -1;
> > - num_insns = 0;
> > max_insns = tb->cflags & CF_COUNT_MASK;
> > - if (max_insns == 0)
> > + if (max_insns == 0) {
> > max_insns = CF_COUNT_MASK;
> > -
> > + }
> > gen_tb_start();
> > for(;;) {
> > if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) {
> > @@ -8338,9 +8337,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > if (lj < j) {
> > - lj++;
> > - while (lj < j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj < j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > }
> > tcg_ctx.gen_opc_pc[lj] = pc_ptr;
> > gen_opc_cc_op[lj] = dc->cc_op;
> > @@ -8387,9 +8386,9 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
> > /* we don't forget to fill the last values */
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > - lj++;
> > - while (lj <= j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj <= j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > }
> >
> > #ifdef DEBUG_DISAS
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index b7f8203..d1e5d84 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -15571,10 +15571,10 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
> > #else
> > ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU;
> > #endif
> > - num_insns = 0;
> > max_insns = tb->cflags & CF_COUNT_MASK;
> > - if (max_insns == 0)
> > + if (max_insns == 0) {
> > max_insns = CF_COUNT_MASK;
> > + }
> > LOG_DISAS("\ntb %p idx %d hflags %04x\n", tb, ctx.mem_idx, ctx.hflags);
> > gen_tb_start();
> > while (ctx.bstate == BS_NONE) {
> > @@ -15595,9 +15595,9 @@ gen_intermediate_code_internal (CPUMIPSState *env, TranslationBlock *tb,
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > if (lj < j) {
> > - lj++;
> > - while (lj < j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj < j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > }
> > tcg_ctx.gen_opc_pc[lj] = ctx.pc;
> > gen_opc_hflags[lj] = ctx.hflags & MIPS_HFLAG_BMASK;
> > @@ -15678,9 +15678,9 @@ done_generating:
> > *tcg_ctx.gen_opc_ptr = INDEX_op_end;
> > if (search_pc) {
> > j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
> > - lj++;
> > - while (lj <= j)
> > - tcg_ctx.gen_opc_instr_start[lj++] = 0;
> > + while (++lj <= j) {
> > + tcg_ctx.gen_opc_instr_start[lj] = 0;
> > + }
> > } else {
> > tb->size = ctx.pc - pc_start;
> > tb->icount = num_insns;
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-10 7:55 ` li guang
@ 2013-04-10 9:54 ` Paolo Bonzini
2013-04-11 2:11 ` li guang
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2013-04-10 9:54 UTC (permalink / raw)
To: li guang
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
Il 10/04/2013 09:55, li guang ha scritto:
> 在 2013-04-10三的 09:44 +0200，Paolo Bonzini写道：
>> Il 10/04/2013 05:30, liguang ha scritto:
>>> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
>>> ---
>>> target-arm/translate.c | 17 ++++++++---------
>>> target-i386/translate.c | 17 ++++++++---------
>>> target-mips/translate.c | 16 ++++++++--------
>>> 3 files changed, 24 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>>> index 35a21be..c0c080d 100644
>>> --- a/target-arm/translate.c
>>> +++ b/target-arm/translate.c
>>> @@ -9806,11 +9806,10 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
>>> cpu_M0 = tcg_temp_new_i64();
>>> next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
>>> lj = -1;
>>> - num_insns = 0;
>>
>> Nack. Did you even try to read what I and Wei-Ren Chen were trying to
>> tell you?
>>
>
> well, you ask if I tested, and I answer yes,
> you doubt if this line really could be removed,
> and I said compiler will not complain this.
I don't care if the compiler doesn't complain (though I doubt it
doesn't; are you using --enable-debug?). It is wrong.
You are removing the initialization of num_insns. The only instruction
that modifies it is now "num_insns++". That is wrong, period. Even if
GCC ends up producing code that works, what happens when you access
uninitialized memory is undefined.
> what should I do?
> Isn't it enough?
> did you read my answer for your comment?
You didn't reply to this message from Wei-Ren Chen:
>> I think 'max_insns = tb->cflags & CF_COUNT_MASK;' is enough
>> to feed compiler.
>
> num_insns and max_insns are two different variables, right? So this
> assignment does not do anything with num_insns.
So yes, I read your answers and no, they were not enough.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-10 9:54 ` Paolo Bonzini
@ 2013-04-11 2:11 ` li guang
2013-04-11 6:23 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: li guang @ 2013-04-11 2:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
在 2013-04-10三的 11:54 +0200，Paolo Bonzini写道：
> Il 10/04/2013 09:55, li guang ha scritto:
> > 在 2013-04-10三的 09:44 +0200，Paolo Bonzini写道：
> >> Il 10/04/2013 05:30, liguang ha scritto:
> >>> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> >>> ---
> >>> target-arm/translate.c | 17 ++++++++---------
> >>> target-i386/translate.c | 17 ++++++++---------
> >>> target-mips/translate.c | 16 ++++++++--------
> >>> 3 files changed, 24 insertions(+), 26 deletions(-)
> >>>
> >>> diff --git a/target-arm/translate.c b/target-arm/translate.c
> >>> index 35a21be..c0c080d 100644
> >>> --- a/target-arm/translate.c
> >>> +++ b/target-arm/translate.c
> >>> @@ -9806,11 +9806,10 @@ static inline void gen_intermediate_code_internal(CPUARMState *env,
> >>> cpu_M0 = tcg_temp_new_i64();
> >>> next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> >>> lj = -1;
> >>> - num_insns = 0;
> >>
> >> Nack. Did you even try to read what I and Wei-Ren Chen were trying to
> >> tell you?
> >>
> >
> > well, you ask if I tested, and I answer yes,
> > you doubt if this line really could be removed,
> > and I said compiler will not complain this.
>
> I don't care if the compiler doesn't complain (though I doubt it
> doesn't; are you using --enable-debug?
absolutely yes.
> ). It is wrong.
>
> You are removing the initialization of num_insns. The only instruction
> that modifies it is now "num_insns++". That is wrong, period. Even if
> GCC ends up producing code that works, what happens when you access
> uninitialized memory is undefined.
>
> > what should I do?
> > Isn't it enough?
> > did you read my answer for your comment?
>
> You didn't reply to this message from Wei-Ren Chen:
>
> >> I think 'max_insns = tb->cflags & CF_COUNT_MASK;' is enough
> >> to feed compiler.
> >
> > num_insns and max_insns are two different variables, right? So this
> > assignment does not do anything with num_insns.
>
> So yes, I read your answers and no, they were not enough.
OK, I won't change here for next post.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-11 2:11 ` li guang
@ 2013-04-11 6:23 ` Paolo Bonzini
2013-04-11 6:42 ` li guang
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2013-04-11 6:23 UTC (permalink / raw)
To: li guang
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
Il 11/04/2013 04:11, li guang ha scritto:
>> > I don't care if the compiler doesn't complain (though I doubt it
>> > doesn't; are you using --enable-debug?
> absolutely yes.
>
Ok, here is the problem.
--enable-debug disables optimization. It will disable some warnings
that the compiler only produces with -Ox.
You can use --enable-debug when developing, but you must test your
patches without it before submitting.
Problem solved! Thanks,
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal
2013-04-11 6:23 ` Paolo Bonzini
@ 2013-04-11 6:42 ` li guang
0 siblings, 0 replies; 7+ messages in thread
From: li guang @ 2013-04-11 6:42 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, proljc, e.voevodin, qemu-devel, blauwirbel, paul,
afaerber, aurelien, rth
Yes, Thanks!
在 2013-04-11四的 08:23 +0200，Paolo Bonzini写道：
> Il 11/04/2013 04:11, li guang ha scritto:
> >> > I don't care if the compiler doesn't complain (though I doubt it
> >> > doesn't; are you using --enable-debug?
> > absolutely yes.
> >
>
> Ok, here is the problem.
>
> --enable-debug disables optimization. It will disable some warnings
> that the compiler only produces with -Ox.
>
> You can use --enable-debug when developing, but you must test your
> patches without it before submitting.
>
> Problem solved! Thanks,
>
> Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-11 6:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-10 3:30 [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal liguang
2013-04-10 7:44 ` Paolo Bonzini
2013-04-10 7:55 ` li guang
2013-04-10 9:54 ` Paolo Bonzini
2013-04-11 2:11 ` li guang
2013-04-11 6:23 ` Paolo Bonzini
2013-04-11 6:42 ` li guang
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).