Re: [PATCH] target/sh4: Emit insn_start for each insn in gUSA region

[Philippe Mathieu-Daudé]

On 3/6/23 18:55, Richard Henderson wrote:
> Fixes an assert in tcg_gen_code that we don't accidentally
> eliminate an insn_start during optimization.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Test case is tests/tcg/multiarch/testthread.c; the assert for
> equality is new with
>
> https://lore.kernel.org/qemu-devel/20230531040330.8950-26-richard.henderson@linaro.org/

Cc'ing Anton.

> r~
> ---
>   target/sh4/translate.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index efd889d9d3..49c87d7a01 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -2144,9 +2144,7 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State 
> *env)
>   
>       /* The entire region has been translated.  */
>       ctx->envflags &= ~TB_FLAG_GUSA_MASK;
> -    ctx->base.pc_next = pc_end;
> -    ctx->base.num_insns += max_insns - 1;
> -    return;
> +    goto done;
>   
>    fail:
>       qemu_log_mask(LOG_UNIMP, "Unrecognized gUSA sequence %08x-%08x\n",
> @@ -2163,8 +2161,19 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State 
> *env)
>          purposes of accounting within the TB.  We might as well report the
>          entire region consumed via ctx->base.pc_next so that it's immediately
>          available in the disassembly dump.  */
> +
> + done:
>       ctx->base.pc_next = pc_end;
>       ctx->base.num_insns += max_insns - 1;
> +
> +    /*
> +     * Emit insn_start to cover each of the insns in the region.
> +     * This matches an assert in tcg.c making sure that we have
> +     * tb->icount * insn_start.
> +     */
> +    for (i = 1; i < max_insns; ++i) {
> +        tcg_gen_insn_start(pc + i * 2, ctx->envflags);
> +    }
>   }
>   #endif
>