Re: [Qemu-devel] [PATCH 19/26] target-xtensa: implement loop option

[Richard Henderson]

On 05/17/2011 03:32 PM, Max Filippov wrote:
> +    if (env->sregs[LEND] != v) {
> +        tb_invalidate_phys_page_range(
> +                env->sregs[LEND] - 1, env->sregs[LEND], 0);
> +        env->sregs[LEND] = v;
> +        tb_invalidate_phys_page_range(
> +                env->sregs[LEND] - 1, env->sregs[LEND], 0);
> +    }

Why are you invalidating twice?

> +static void gen_check_loop_end(DisasContext *dc, int slot)
> +{
> +    if (option_enabled(dc, XTENSA_OPTION_LOOP) &&
> +            !(dc->tb->flags & XTENSA_TBFLAG_EXCM) &&
> +            dc->next_pc == dc->lend) {
> +        int label = gen_new_label();
> +
> +        tcg_gen_brcondi_i32(TCG_COND_NE, cpu_SR[LEND], dc->next_pc, label);
> +        tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_SR[LCOUNT], 0, label);
> +        tcg_gen_subi_i32(cpu_SR[LCOUNT], cpu_SR[LCOUNT], 1);
> +        gen_jump(dc, cpu_SR[LBEG]);
> +        gen_set_label(label);
> +        gen_jumpi(dc, dc->next_pc, slot);

If you're going to pretend that LEND is a constant, you might as well
pretend that LBEG is also a constant, so that you get to chain the TB's
around the loop.

> +static void gen_jumpi_check_loop_end(DisasContext *dc, int slot)
> +{
> +    gen_check_loop_end(dc, slot);
> +    gen_jumpi(dc, dc->next_pc, slot);

You're generating duplicate jumpi's here; that can probably be avoided
fairly easily.


r~