Re: [PATCH v3 4/9] target/nios2: Add DisasContextBase to DisasContext

[Peter Maydell]

On Mon, 28 Jun 2021 at 23:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Migrate the is_jmp, tb and singlestep_enabled fields from
> DisasContext into the base.  Use pc_first instead of tb->pc.
> Increment pc_next prior to decode, leaving the address of
> the current insn in dc->pc.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/translate.c | 70 +++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 34 deletions(-)
>

>
>      /* Indicate where the next block should start */
> -    switch (dc->is_jmp) {
> +    switch (dc->base.is_jmp) {
>      case DISAS_NEXT:
>      case DISAS_UPDATE:
>          /* Save the current PC back into the CPU register */
> -        tcg_gen_movi_tl(cpu_R[R_PC], dc->pc);
> +        tcg_gen_movi_tl(cpu_R[R_PC], dc->base.pc_next);
>          tcg_gen_exit_tb(NULL, 0);
>          break;
>
> @@ -883,15 +885,15 @@ void gen_intermediate_code(CPUState *cs, 
> TranslationBlock *tb, int max_insns)
>      gen_tb_end(tb, num_insns);
>
>      /* Mark instruction starts for the final generated instruction */
> -    tb->size = dc->pc - tb->pc;
> +    tb->size = dc->pc - dc->base.pc_first;

Shouldn't this one be "dc->base.pc_next - dc->base.pc_first" ?

>      tb->icount = num_insns;
>
>  #ifdef DEBUG_DISAS
>      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> -        && qemu_log_in_addr_range(tb->pc)) {
> +        && qemu_log_in_addr_range(dc->base.pc_first)) {
>          FILE *logfile = qemu_log_lock();
> -        qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> -        log_target_disas(cs, tb->pc, dc->pc - tb->pc);
> +        qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> +        log_target_disas(cs, tb->pc, dc->base.pc_next - dc->base.pc_first);

Here you could use tb->size for the 3rd argument (which
makes it clearer that the arguments are right -- we disassemble
the whole size of the TB starting at its first PC value).

>          qemu_log("\n");
>          qemu_log_unlock(logfile);
>      }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM