All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tcg: Restart code generation when we run out of temps
@ 2021-01-23 23:01 Richard Henderson
  2021-01-24  0:27 ` Philippe Mathieu-Daudé
  2021-01-24 13:48 ` BALATON Zoltan
  0 siblings, 2 replies; 3+ messages in thread
From: Richard Henderson @ 2021-01-23 23:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: lvivier, alistair23

Some large translation blocks can generate so many unique
constants that we run out of temps to hold them.  In this
case, longjmp back to the start of code generation and
restart with a smaller translation block.

Buglink: https://bugs.launchpad.net/bugs/1912065
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

This replaces both the patch to increase the number of temps,
and the buggy patch set that dynamically allocated the temps.


r~

---
 include/tcg/tcg.h         |  3 +++
 accel/tcg/translate-all.c | 15 ++++++++++++++-
 tcg/tcg.c                 | 11 ++++++++---
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index c5a9d65d5f..0f0695e90d 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -680,6 +680,9 @@ struct TCGContext {
 
     uint16_t gen_insn_end_off[TCG_MAX_INSNS];
     target_ulong gen_insn_data[TCG_MAX_INSNS][TARGET_INSN_START_WORDS];
+
+    /* Exit to translator on overflow. */
+    sigjmp_buf jmp_trans;
 };
 
 static inline bool temp_readonly(TCGTemp *ts)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d09c187e0f..81d4c83f22 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1926,11 +1926,17 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     ti = profile_getclock();
 #endif
 
+    gen_code_size = sigsetjmp(tcg_ctx->jmp_trans, 0);
+    if (unlikely(gen_code_size != 0)) {
+        goto error_return;
+    }
+
     tcg_func_start(tcg_ctx);
 
     tcg_ctx->cpu = env_cpu(env);
     gen_intermediate_code(cpu, tb, max_insns);
     tcg_ctx->cpu = NULL;
+    max_insns = tb->icount;
 
     trace_translate_block(tb, tb->pc, tb->tc.ptr);
 
@@ -1955,6 +1961,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 
     gen_code_size = tcg_gen_code(tcg_ctx, tb);
     if (unlikely(gen_code_size < 0)) {
+ error_return:
         switch (gen_code_size) {
         case -1:
             /*
@@ -1966,6 +1973,9 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
              * flush the TBs, allocate a new TB, re-initialize it per
              * above, and re-do the actual code generation.
              */
+            qemu_log_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT,
+                          "Restarting code generation for "
+                          "code_gen_buffer overflow\n");
             goto buffer_overflow;
 
         case -2:
@@ -1978,9 +1988,12 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
              * Try again with half as many insns as we attempted this time.
              * If a single insn overflows, there's a bug somewhere...
              */
-            max_insns = tb->icount;
             assert(max_insns > 1);
             max_insns /= 2;
+            qemu_log_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT,
+                          "Restarting code generation with "
+                          "smaller translation block (max %d insns)\n",
+                          max_insns);
             goto tb_overflow;
 
         default:
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 67b08f708d..9e1b0d73c7 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1205,18 +1205,23 @@ void tcg_func_start(TCGContext *s)
     QSIMPLEQ_INIT(&s->labels);
 }
 
-static inline TCGTemp *tcg_temp_alloc(TCGContext *s)
+static TCGTemp *tcg_temp_alloc(TCGContext *s)
 {
     int n = s->nb_temps++;
-    tcg_debug_assert(n < TCG_MAX_TEMPS);
+
+    if (n >= TCG_MAX_TEMPS) {
+        /* Signal overflow, starting over with fewer guest insns. */
+        siglongjmp(s->jmp_trans, -2);
+    }
     return memset(&s->temps[n], 0, sizeof(TCGTemp));
 }
 
-static inline TCGTemp *tcg_global_alloc(TCGContext *s)
+static TCGTemp *tcg_global_alloc(TCGContext *s)
 {
     TCGTemp *ts;
 
     tcg_debug_assert(s->nb_globals == s->nb_temps);
+    tcg_debug_assert(s->nb_globals < TCG_MAX_TEMPS);
     s->nb_globals++;
     ts = tcg_temp_alloc(s);
     ts->kind = TEMP_GLOBAL;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tcg: Restart code generation when we run out of temps
  2021-01-23 23:01 [PATCH] tcg: Restart code generation when we run out of temps Richard Henderson
@ 2021-01-24  0:27 ` Philippe Mathieu-Daudé
  2021-01-24 13:48 ` BALATON Zoltan
  1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-01-24  0:27 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: lvivier, alistair23

On 1/24/21 12:01 AM, Richard Henderson wrote:
> Some large translation blocks can generate so many unique
> constants that we run out of temps to hold them.  In this
> case, longjmp back to the start of code generation and
> restart with a smaller translation block.

Clever and way nicer.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Buglink: https://bugs.launchpad.net/bugs/1912065
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> This replaces both the patch to increase the number of temps,
> and the buggy patch set that dynamically allocated the temps.
> 
> 
> r~
> 
> ---
>  include/tcg/tcg.h         |  3 +++
>  accel/tcg/translate-all.c | 15 ++++++++++++++-
>  tcg/tcg.c                 | 11 ++++++++---
>  3 files changed, 25 insertions(+), 4 deletions(-)


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tcg: Restart code generation when we run out of temps
  2021-01-23 23:01 [PATCH] tcg: Restart code generation when we run out of temps Richard Henderson
  2021-01-24  0:27 ` Philippe Mathieu-Daudé
@ 2021-01-24 13:48 ` BALATON Zoltan
  1 sibling, 0 replies; 3+ messages in thread
From: BALATON Zoltan @ 2021-01-24 13:48 UTC (permalink / raw)
  To: Richard Henderson; +Cc: lvivier, alistair23, qemu-devel

On Sat, 23 Jan 2021, Richard Henderson wrote:
> Some large translation blocks can generate so many unique
> constants that we run out of temps to hold them.  In this
> case, longjmp back to the start of code generation and
> restart with a smaller translation block.
>
> Buglink: https://bugs.launchpad.net/bugs/1912065
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

> ---
>
> This replaces both the patch to increase the number of temps,
> and the buggy patch set that dynamically allocated the temps.
>
>
> r~
>
> ---
> include/tcg/tcg.h         |  3 +++
> accel/tcg/translate-all.c | 15 ++++++++++++++-
> tcg/tcg.c                 | 11 ++++++++---
> 3 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index c5a9d65d5f..0f0695e90d 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -680,6 +680,9 @@ struct TCGContext {
>
>     uint16_t gen_insn_end_off[TCG_MAX_INSNS];
>     target_ulong gen_insn_data[TCG_MAX_INSNS][TARGET_INSN_START_WORDS];
> +
> +    /* Exit to translator on overflow. */
> +    sigjmp_buf jmp_trans;
> };
>
> static inline bool temp_readonly(TCGTemp *ts)
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index d09c187e0f..81d4c83f22 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1926,11 +1926,17 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>     ti = profile_getclock();
> #endif
>
> +    gen_code_size = sigsetjmp(tcg_ctx->jmp_trans, 0);
> +    if (unlikely(gen_code_size != 0)) {
> +        goto error_return;
> +    }
> +
>     tcg_func_start(tcg_ctx);
>
>     tcg_ctx->cpu = env_cpu(env);
>     gen_intermediate_code(cpu, tb, max_insns);
>     tcg_ctx->cpu = NULL;
> +    max_insns = tb->icount;
>
>     trace_translate_block(tb, tb->pc, tb->tc.ptr);
>
> @@ -1955,6 +1961,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>
>     gen_code_size = tcg_gen_code(tcg_ctx, tb);
>     if (unlikely(gen_code_size < 0)) {
> + error_return:
>         switch (gen_code_size) {
>         case -1:
>             /*
> @@ -1966,6 +1973,9 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>              * flush the TBs, allocate a new TB, re-initialize it per
>              * above, and re-do the actual code generation.
>              */
> +            qemu_log_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT,
> +                          "Restarting code generation for "
> +                          "code_gen_buffer overflow\n");
>             goto buffer_overflow;
>
>         case -2:
> @@ -1978,9 +1988,12 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>              * Try again with half as many insns as we attempted this time.
>              * If a single insn overflows, there's a bug somewhere...
>              */
> -            max_insns = tb->icount;
>             assert(max_insns > 1);
>             max_insns /= 2;
> +            qemu_log_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT,
> +                          "Restarting code generation with "
> +                          "smaller translation block (max %d insns)\n",
> +                          max_insns);
>             goto tb_overflow;
>
>         default:
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 67b08f708d..9e1b0d73c7 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1205,18 +1205,23 @@ void tcg_func_start(TCGContext *s)
>     QSIMPLEQ_INIT(&s->labels);
> }
>
> -static inline TCGTemp *tcg_temp_alloc(TCGContext *s)
> +static TCGTemp *tcg_temp_alloc(TCGContext *s)
> {
>     int n = s->nb_temps++;
> -    tcg_debug_assert(n < TCG_MAX_TEMPS);
> +
> +    if (n >= TCG_MAX_TEMPS) {
> +        /* Signal overflow, starting over with fewer guest insns. */
> +        siglongjmp(s->jmp_trans, -2);
> +    }
>     return memset(&s->temps[n], 0, sizeof(TCGTemp));
> }
>
> -static inline TCGTemp *tcg_global_alloc(TCGContext *s)
> +static TCGTemp *tcg_global_alloc(TCGContext *s)
> {
>     TCGTemp *ts;
>
>     tcg_debug_assert(s->nb_globals == s->nb_temps);
> +    tcg_debug_assert(s->nb_globals < TCG_MAX_TEMPS);
>     s->nb_globals++;
>     ts = tcg_temp_alloc(s);
>     ts->kind = TEMP_GLOBAL;
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-24 13:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-23 23:01 [PATCH] tcg: Restart code generation when we run out of temps Richard Henderson
2021-01-24  0:27 ` Philippe Mathieu-Daudé
2021-01-24 13:48 ` BALATON Zoltan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.