Re: [Qemu-devel] [PATCH 11/12] target-arm: A64: add support for 'test and branch' imm

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> +    label_nomatch = gen_new_label();
> +    if (op) { /* TBNZ */
> +        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
> +    } else { /* TBZ */
> +        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
> +    }
> +    tcg_temp_free_i64(tcg_cmp);
> +    gen_goto_tb(s, 0, addr);
> +    gen_set_label(label_nomatch);
> +    gen_goto_tb(s, 1, s->pc);

Similar to B.cond, I think it would be clearer to write positive tests than
negative tests.  Therefore TBNZ -> NE; TBZ -> EQ, and swap the two goto_tb.

Further, you can avoid some repetition of the brcondi with (?:).

Otherwise,

Reviewed-by: Richard Henderson <address@hidden>


r~