| Message ID | 1365564652-26981-1-git-send-email-lig.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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; >
在 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; > > >
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
在 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!
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
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
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;
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(-)