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

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

[Richard Henderson]

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/


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
 
-- 
2.34.1

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
>   

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/
> 
> 
> r~
> ---
>   target/sh4/translate.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

[Richard Henderson]

On 6/5/23 08:01, Philippe Mathieu-Daudé wrote:
> 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/
>>
>>
>> r~
>> ---
>>   target/sh4/translate.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

queued to tcg-next.

r~