On Thu, 17 Mar 2022 at 05:09, Richard Henderson
<[email protected]> wrote:
>
> With TCG_OPF_COND_BRANCH, we extended the lifetimes of
> globals across extended basic blocks. This means that
> the liveness computed in pass 1 does not kill globals
> in the same way as normal temps.
>
> Introduce TYPE_EBB to match this lifetime, so that we
> get correct register allocation for the temps that we
> introduce during the indirect lowering pass.
>
> Fixes: b4cb76e6208 ("tcg: Do not kill globals at conditional branches")
> Signed-off-by: Richard Henderson <[email protected]>
> ---
> include/tcg/tcg.h | 2 ++
> tcg/tcg.c | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index 73869fd9d0..27de13fae0 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -433,6 +433,8 @@ typedef enum TCGTempVal {
> typedef enum TCGTempKind {
> /* Temp is dead at the end of all basic blocks. */
> TEMP_NORMAL,
> + /* Temp is live across conditional branch, but dead otherwise. */
> + TEMP_EBB,
> /* Temp is saved across basic blocks but dead at the end of TBs. */
> TEMP_LOCAL,
> /* Temp is saved across both basic blocks and translation blocks. */
Maybe add an assert() in tcg_temp_free_internal() that ts->kind is
not TEMP_EBB ? (This was about the only place in the file that
does different things based on ts->kind that you haven't added
TEMP_EBB handling for.)
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 33a97eabdb..45030e88fd 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1674,6 +1674,7 @@ static void tcg_reg_alloc_start(TCGContext *s)
> case TEMP_GLOBAL:
> break;
> case TEMP_NORMAL:
> + case TEMP_EBB:
> val = TEMP_VAL_DEAD;
> /* fall through */
> case TEMP_LOCAL:
> @@ -1701,6 +1702,9 @@ static char *tcg_get_arg_str_ptr(TCGContext *s, char
> *buf, int buf_size,
> case TEMP_LOCAL:
> snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
> break;
> + case TEMP_EBB:
> + snprintf(buf, buf_size, "ebb%d", idx - s->nb_globals);
> + break;
> case TEMP_NORMAL:
> snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
> break;
> @@ -2378,6 +2382,7 @@ static void la_bb_end(TCGContext *s, int ng, int nt)
> state = TS_DEAD | TS_MEM;
> break;
> case TEMP_NORMAL:
> + case TEMP_EBB:
> case TEMP_CONST:
> state = TS_DEAD;
> break;
> @@ -2427,6 +2432,7 @@ static void la_bb_sync(TCGContext *s, int ng, int nt)
> case TEMP_NORMAL:
> s->temps[i].state = TS_DEAD;
> break;
> + case TEMP_EBB:
> case TEMP_CONST:
> continue;
> default:
The comment on la_bb_sync() needs updating:
/*
* liveness analysis: conditional branch: all temps are dead
* unless explicitly live-across-conditional-branch, globals
* and local temps should be synced.
*/
> @@ -2797,6 +2803,7 @@ static bool liveness_pass_2(TCGContext *s)
> TCGTemp *dts = tcg_temp_alloc(s);
> dts->type = its->type;
> dts->base_type = its->base_type;
> + dts->kind = TEMP_EBB;
> its->state_ptr = dts;
> } else {
> its->state_ptr = NULL;
> @@ -3107,6 +3114,7 @@ static void temp_free_or_dead(TCGContext *s, TCGTemp
> *ts, int free_or_dead)
> new_type = TEMP_VAL_MEM;
> break;
> case TEMP_NORMAL:
> + case TEMP_EBB:
> new_type = free_or_dead < 0 ? TEMP_VAL_MEM : TEMP_VAL_DEAD;
> break;
> case TEMP_CONST:
> @@ -3353,6 +3361,7 @@ static void tcg_reg_alloc_bb_end(TCGContext *s,
> TCGRegSet allocated_regs)
> temp_save(s, ts, allocated_regs);
> break;
> case TEMP_NORMAL:
> + case TEMP_EBB:
> /* The liveness analysis already ensures that temps are dead.
> Keep an tcg_debug_assert for safety. */
> tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
> @@ -3390,6 +3399,7 @@ static void tcg_reg_alloc_cbranch(TCGContext *s,
> TCGRegSet allocated_regs)
> case TEMP_NORMAL:
> tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
> break;
> + case TEMP_EBB:
> case TEMP_CONST:
> break;
> default:
Similarly, the comment above tcg_reg_alloc_cbranch() now should
say "all temporaries are dead unless explicitly
live-across-conditional-branch".
Otherwise
Reviewed-by: Peter Maydell <[email protected]>
(though review from somebody more familiar with the TCG internals
than me would still be useful I think)
thanks
-- PMM
