Re: [PATCH v14 04/22] cpu: Move synchronize_from_tb() to tcg_ops

[Richard Henderson <richard.henderson@linaro.org>]

On 1/28/21 8:58 AM, Alex Bennée wrote:
> Looking at the function here I wonder if we should be worried about the
> thumb state? Peter?
> 
> static void arm_cpu_set_pc(CPUState *cs, vaddr value)
> {
>     ARMCPU *cpu = ARM_CPU(cs);
>     CPUARMState *env = &cpu->env;
> 
>     if (is_a64(env)) {
>         env->pc = value;
>         env->thumb = 0;
>     } else {
>         env->regs[15] = value & ~1;
>         env->thumb = value & 1;
>     }
> }

Plausible.  You could possibly test this via gdbstub, as there are not many
other users.  I think it would be of the form:

(gdb) call foo()

where foo is a thumb function.

> #ifdef CONFIG_TCG
> void arm_cpu_synchronize_from_tb(CPUState *cs,
>                                  const TranslationBlock *tb)
> {
>     ARMCPU *cpu = ARM_CPU(cs);
>     CPUARMState *env = &cpu->env;
> 
>     /*
>      * It's OK to look at env for the current mode here, because it's
>      * never possible for an AArch64 TB to chain to an AArch32 TB.
>      */
>     if (is_a64(env)) {
>         env->pc = tb->pc;
>     } else {
>         env->regs[15] = tb->pc;
>     }
> }
> #endif /* CONFIG_TCG */

This function need only handle any state that is "deferred" across goto_tb.
This is almost always simply the pc, e.g.

    if (use_goto_tb(s, dest)) {
        tcg_gen_goto_tb(n);
        gen_set_pc_im(s, dest);
        tcg_gen_exit_tb(s->base.tb, n);

A few targets do a bit more than that, especially vs delayed branches, but ARM
does not.

But there should be no thumb state that ought to be updated here.


r~