Re: [PATCH] Correct CRIS TCG register lifetime management

[Richard Henderson]

On 2/18/21 12:56 AM, Stefan Sandström wrote:
> From: Stefan Sandstrom <stefans@axis.com>
> 
> Add and fix deallocation of temporary TCG registers in CRIS code
> generation.
> 
> Change-Id: I17fce5d95bdc4418337ba885d53ba97afb1bafcc
> Signed-off-by: Stefan Sandstrom <stefans@axis.com>
> ---
> target/cris/translate.c         | 146 ++++++++++++++++++++++++++++++----------
> target/cris/translate_v10.c.inc |  70 ++++++++++++-------
> 2 files changed, 156 insertions(+), 60 deletions(-)
> 
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index c893f87..ae903a5 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -177,9 +177,13 @@ static inline void t_gen_mov_TN_preg(TCGv tn, int r)
> {
>     assert(r >= 0 && r <= 15);
>     if (r == PR_BZ || r == PR_WZ || r == PR_DZ) {
> -        tcg_gen_mov_tl(tn, tcg_const_tl(0));
> +        TCGv c0 = tcg_const_tl(0);
> +        tcg_gen_mov_tl(tn, c0);
> +        tcg_temp_free(c0);

In cases like this, you should just use tcg_gen_movi_tl(tn, 0).

>     } else if (r == PR_VR) {
> -        tcg_gen_mov_tl(tn, tcg_const_tl(32));
> +        TCGv c32 = tcg_const_tl(32);
> +        tcg_gen_mov_tl(tn, c32);
> +        tcg_temp_free(c32);

movi

> static void cris_lock_irq(DisasContext *dc)
> {
> +    TCGv c1 = tcg_const_tl(1);
>     dc->clear_locked_irq = 0;
> -    t_gen_mov_env_TN(locked_irq, tcg_const_tl(1));
> +    t_gen_mov_env_TN(locked_irq, c1);
> +    tcg_temp_free(c1);
> }

good.

> @@ -885,8 +891,10 @@ static void gen_tst_cc (DisasContext *dc, TCGv cc, int 
> cond)
>     case CC_EQ:
>         if ((arith_opt || move_opt)
>                 && dc->cc_x_uptodate != (2 | X_FLAG)) {
> +            TCGv c0 = tcg_const_tl(0);
>             tcg_gen_setcond_tl(TCG_COND_EQ, cc,
> -                    cc_result, tcg_const_tl(0));
> +                    cc_result, c0);
> +            tcg_temp_free(c0);

setcondi

> -    tcg_gen_shl_tl(t0, cpu_R[dc->op2], tcg_const_tl(dc->zzsize));
> +    c = tcg_const_tl(dc->zzsize);
> +    tcg_gen_shl_tl(t0, cpu_R[dc->op2], c);

shli

> @@ -3023,14 +3083,16 @@ static unsigned int crisv32_decoder(CPUCRISState 
> *env, DisasContext *dc)
>     /* Single-stepping ?  */
>     if (dc->tb_flags & S_FLAG) {
>         TCGLabel *l1 = gen_new_label();
> +        TCGv c = tcg_const_tl(3);
>         tcg_gen_brcondi_tl(TCG_COND_NE, cpu_PR[PR_SPC], dc->pc, l1);
>         /* We treat SPC as a break with an odd trap vector.  */
>         cris_evaluate_flags(dc);
> -        t_gen_mov_env_TN(trap_vector, tcg_const_tl(3));
> +        t_gen_mov_env_TN(trap_vector, c);

Here, you cannot lift the const allocation above the branch.
There are enough uses of t_gen_mov_env_TN with a constant that it would be
worthwhile creating a new function that takes the constant.


r~