Re: [Qemu-devel] [PATCH] translate: cleanup gen_intermediate_code_internal

[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 <address@hidden>
>>> ---
>>>  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