[v1] tcg: Reset free_temps before tcg_optimize

[PATCH] tcg: Reset free_temps before tcg_optimize

Posted by Richard Henderson 1 year, 2 months ago

When allocating new temps during tcg_optmize, do not re-use
any EBB temps that were used within the TB.  We do not have
any idea what span of the TB in which the temp was live.

Cc: qemu-stable@nongnu.org
Fixes: fb04ab7ddd8 ("tcg/optimize: Lower TCG_COND_TST{EQ,NE} if unsupported")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2711
Reported-by: wannacu <wannacu2049@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Unless there's some other reason to spin an -rc4, this can wait
to be the first patch for 10.0.1.

r~

---
 tcg/tcg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0babae1b88..eece825e2e 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -6120,6 +6120,9 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb, uint64_t pc_start)
     }
 #endif
 
+    /* Do not reuse any EBB that may be allocated within the TB. */
+    memset(s->free_temps, 0, sizeof(s->free_temps));
+
     tcg_optimize(s);
 
     reachable_code_pass(s);
-- 
2.43.0

Re: [PATCH] tcg: Reset free_temps before tcg_optimize

Posted by Philippe Mathieu-Daudé 1 year, 2 months ago

On 7/12/24 22:47, Richard Henderson wrote:
> When allocating new temps during tcg_optmize, do not re-use
> any EBB temps that were used within the TB.  We do not have
> any idea what span of the TB in which the temp was live.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: fb04ab7ddd8 ("tcg/optimize: Lower TCG_COND_TST{EQ,NE} if unsupported")

Hmm, the problem is due to arg_new_temp()?

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2711
> Reported-by: wannacu <wannacu2049@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Unless there's some other reason to spin an -rc4, this can wait
> to be the first patch for 10.0.1.
> 
> r~
> 
> ---
>   tcg/tcg.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 0babae1b88..eece825e2e 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -6120,6 +6120,9 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb, uint64_t pc_start)
>       }
>   #endif
>   
> +    /* Do not reuse any EBB that may be allocated within the TB. */
> +    memset(s->free_temps, 0, sizeof(s->free_temps));

Maybe add as tcg_temp_[ebb]_[reset|flush]() helper so we can reuse,
like in tcg_func_start() and plugin_gen_inject()?

>       tcg_optimize(s);
>   
>       reachable_code_pass(s);

Re: [PATCH] tcg: Reset free_temps before tcg_optimize

Posted by Richard Henderson 1 year, 2 months ago

On 12/8/24 12:01, Philippe Mathieu-Daudé wrote:
> On 7/12/24 22:47, Richard Henderson wrote:
>> When allocating new temps during tcg_optmize, do not re-use
>> any EBB temps that were used within the TB.  We do not have
>> any idea what span of the TB in which the temp was live.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: fb04ab7ddd8 ("tcg/optimize: Lower TCG_COND_TST{EQ,NE} if unsupported")
> 
> Hmm, the problem is due to arg_new_temp()?

Yes.

> Maybe add as tcg_temp_[ebb]_[reset|flush]() helper so we can reuse,
> like in tcg_func_start() and plugin_gen_inject()?

Ah, right.  I had a memory of solving this same problem once, but couldn't remember where, 
or if it was on a branch somewhere.  Good idea.


r~

Re: [PATCH] tcg: Reset free_temps before tcg_optimize

Posted by Pierrick Bouvier 1 year, 2 months ago

On 12/7/24 13:47, Richard Henderson wrote:
> When allocating new temps during tcg_optmize, do not re-use
> any EBB temps that were used within the TB.  We do not have
> any idea what span of the TB in which the temp was live.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: fb04ab7ddd8 ("tcg/optimize: Lower TCG_COND_TST{EQ,NE} if unsupported")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2711
> Reported-by: wannacu <wannacu2049@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Unless there's some other reason to spin an -rc4, this can wait
> to be the first patch for 10.0.1.
> 
> r~
> 
> ---
>   tcg/tcg.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 0babae1b88..eece825e2e 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -6120,6 +6120,9 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb, uint64_t pc_start)
>       }
>   #endif
>   
> +    /* Do not reuse any EBB that may be allocated within the TB. */
> +    memset(s->free_temps, 0, sizeof(s->free_temps));
> +
>       tcg_optimize(s);
>   
>       reachable_code_pass(s);

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>