Re: [PATCH for-8.1] target/sparc: Use tcg_gen_lookup_and_goto_ptr

[Philippe Mathieu-Daudé]

On 5/4/23 20:59, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/translate.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 137bdc5159..47940fd85e 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -322,7 +322,7 @@ static void gen_goto_tb(DisasContext *s, int tb_num,

[expanding diff context]

        if (use_goto_tb(s, pc, npc))  {
            /* jump to same page: we can use a direct jump */
            tcg_gen_goto_tb(tb_num);
            tcg_gen_movi_tl(cpu_pc, pc);
            tcg_gen_movi_tl(cpu_npc, npc);
            tcg_gen_exit_tb(s->base.tb, tb_num);
        } else {

>           /* jump to another page: currently not optimized */
>           tcg_gen_movi_tl(cpu_pc, pc);
>           tcg_gen_movi_tl(cpu_npc, npc);
> -        tcg_gen_exit_tb(NULL, 0);
> +        tcg_gen_lookup_and_goto_ptr();

Per 
https://qemu.readthedocs.io/en/latest/devel/tcg.html#lookup-and-goto-ptr 
[*]:

This helper will look for an existing TB that matches the current CPU 
state. If the destination TB is available its code address is returned, 
otherwise the address of the JIT epilogue is returned.

OK. IIUC this is the "optimized" form (trying to not exit the current
TB). Should the comment be updated to "/* jump to another page */"?

>       }
>   }
>   
> @@ -4153,7 +4153,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)

[expanding diff context]

                             case 0x3: /* V9 wrasi */

wrasi = "Write ASI register" instruction.

>                                   /* End TB to notice changed ASI.  */
>                                   save_state(dc);
>                                   gen_op_next_insn();
> -                                tcg_gen_exit_tb(NULL, 0);
> +                                tcg_gen_lookup_and_goto_ptr();

Memory mapping is not changed, CPU state change is constant,
no interrupt updated, OK.

>                                   dc->base.is_jmp = DISAS_NORETURN;
>                                   break;
>                               case 0x6: /* V9 wrfprs */

wrfprs = "Write floating-point registers state register".

>                                   dc->fprs_dirty = 0;
>                                   save_state(dc);
>                                   gen_op_next_insn();
> -                                tcg_gen_exit_tb(NULL, 0);
> +                                tcg_gen_lookup_and_goto_ptr();

Similar analysis, OK.

>                                   dc->base.is_jmp = DISAS_NORETURN;
>                                   break;
>                               case 0xf: /* V9 sir, nop if user */
> @@ -5661,7 +5661,7 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, 
> CPUState *cs)

[expanding diff context]

        switch (dc->base.is_jmp) {
        case DISAS_NEXT:
        case DISAS_TOO_MANY:
            if (dc->pc != DYNAMIC_PC &&
                (dc->npc != DYNAMIC_PC && dc->npc != JUMP_PC)) {
                /* static PC and NPC: we can use direct chaining */
                gen_goto_tb(dc, 0, dc->pc, dc->npc);
            } else {
                if (dc->pc != DYNAMIC_PC) {

>                   tcg_gen_movi_tl(cpu_pc, dc->pc);
>               }
>               save_npc(dc);
> -            tcg_gen_exit_tb(NULL, 0);
> +            tcg_gen_lookup_and_goto_ptr();

Per [*] "we either branch to the next TB or return to the main loop."

So here we just perform an indirect branch, possibly using the JIT
epilogue instead of directly returning to the main loop. OK.

To the best of my knowledge:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>           }
>           break;
>