Re: [Qemu-devel] [PATCH RFC v8 01/12] target/rx: TCG translation

[Yoshinori Sato <ysato@users.sourceforge.jp>]

On Sat, 04 May 2019 03:43:23 +0900,
Richard Henderson wrote:
> 
> On 5/2/19 7:33 AM, Yoshinori Sato wrote:
> > +/* conditional branch helper */
> > +static void rx_bcnd_main(DisasContext *ctx, int cd, int dst)
> > +{
> > +    DisasCompare dc;
> > +    TCGLabel *t, *done;
> > +
> > +    switch (cd) {
> > +    case 0 ... 13:
> > +        dc.temp = tcg_temp_new();
> > +        psw_cond(&dc, cd);
> > +        t = gen_new_label();
> > +        done = gen_new_label();
> > +        tcg_gen_brcondi_i32(dc.cond, dc.value, 0, t);
> > +        gen_goto_tb(ctx, 0, ctx->base.pc_next);
> > +        tcg_gen_br(done);
> > +        gen_set_label(t);
> > +        gen_goto_tb(ctx, 1, ctx->pc + dst);
> > +        gen_set_label(done);
> > +        tcg_temp_free(dc.temp);
> > +        break;
> > +    case 14:
> > +        /* always true case */
> > +        gen_goto_tb(ctx, 0, ctx->pc + dst);
> > +        break;
> > +    case 15:
> > +        /* always false case */
> > +        /* Nothing do */
> > +        break;
> > +    }
> > +    ctx->base.is_jmp = DISAS_JUMP;
> > +}
> 
> Do not set is_jmp to DISAS_JUMP here.  We have already set is_jmp to
> DISAS_NORETURN in gen_goto_tb.  For case 15, we do not need to exit the TB in
> order to treat the never-taken branch as a nop.
> 
> This assignment means that we will emit *another* exit from the TB in
> rx_tr_tb_stop, which will be unreachable code.
> 
> This is the only bug I see in this revision.  Thanks for your patience!
>

OK.
I did not notice it because it was never used.

Too many thanks.

> 
> r~
> 

-- 
Yosinori Sato