On Sun, 25 Jul 2021 at 18:44, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> In cpu_loop_exec_tb(), we decide whether to look for a TB with
> exactly insns_left instructions in it using the condition
> (!cpu->icount_extra && insns_left > 0 && insns_left < tb->icount)
>
> The check for icount_extra == 0 is unnecessary, because we just set
> insns_left = MIN(0xffff, cpu->icount_budget);
> icount_extra = icount_budget - insns_left;
> and so icount_extra can only be non-zero if icount_budget > 0xffff
> and insns_left == 0xffff. But in that case insns_left >= tb->icount
> because 0xffff is much larger than TCG_MAX_INSNS, so the condition
> will be false anyway.
>
> Remove the unnecessary check, and instead assert:
> * that we are only going to execute a partial TB here if the
> icount budget has run out (ie icount_extra == 0)
> * that the number of insns we're going to execute does fit into
> the CF_COUNT_MASK
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> You could argue that we don't need the asserts, if you like.
> The first one would have caught the bug fixed in the previous
> commit, though.
"first" in the bulleted list, "second" in the order I put them in the code...
> ---
> accel/tcg/cpu-exec.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 6e8dc291197..5aa42fbff35 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -843,7 +843,9 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
> TranslationBlock *tb,
> * execute we need to ensure we find/generate a TB with exactly
> * insns_left instructions in it.
> */
> - if (!cpu->icount_extra && insns_left > 0 && insns_left < tb->icount) {
> + if (insns_left > 0 && insns_left < tb->icount) {
> + assert(insns_left <= CF_COUNT_MASK);
> + assert(cpu->icount_extra == 0);
> cpu->cflags_next_tb = (tb->cflags & ~CF_COUNT_MASK) | insns_left;
> }
> #endif
-- PMM
