Re: [Qemu-devel] [PATCH 3/3] target/tricore: Use translate_loop

[Richard Henderson]

On 6/17/19 7:35 AM, Bastian Koppelmann wrote:
> +static void tricore_tr_init_disas_context(DisasContextBase *dcbase,
> +                                          CPUState *cs)
>  {
> +    DisasContext *ctx = container_of(dcbase, DisasContext, base);
>      CPUTriCoreState *env = cs->env_ptr;
> +    ctx->base.pc_next = ctx->base.pc_first;

This is already done in generic code.

I don't see an initialization of hflags & saved_hflags?
Although I don't see that either before or afterward...

> +static bool tricore_tr_breakpoint_check(DisasContextBase *dcbase, CPUState 
> *cpu,
> +                                      const CPUBreakpoint *bp)
> +{
> +    return true;
> +}

Not supporting breakpoints, I think it's better to return false here.

Although it's not difficult -- just raise EXCP_DEBUG as an exception.
It'd be nice to follow up and fix this afterward.

> +static void tricore_tr_translate_insn(DisasContextBase *dcbase, CPUState 
> *cpu)
> +{
> +    DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +    CPUTriCoreState *env = cpu->env_ptr;
> +
> +    ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
> +    decode_opc(ctx);
> +    ctx->base.pc_next = ctx->pc_succ_insn;
> +
> +    if (ctx->base.is_jmp == DISAS_NEXT) {
> +        target_ulong page_start;
> +
> +        page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
> +        if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
> +            ctx->base.is_jmp = DISAS_TOO_MANY;
>          }

This isn't perfect as an ending, but you didn't seem to have one at all before,
so I guess improvements can come incrementally afterward.

Have a look at the end of thumb_tr_translate_insn & insn_crosses_page to see
how to handle this properly.


r~