Re: [Qemu-devel] Confusion regarding temporaries with branch conditional

[Nikunj A Dadhania]

Peter Maydell <address@hidden> writes:

> On 30 November 2016 at 07:00, Nikunj A Dadhania
> <address@hidden> wrote:
>>
>> Hi,
>>
>> I was writing one instruction and hit following issue:
>>
>> [snip]/qemu/tcg/tcg.c:2039: tcg fatal error
>> qemu-ppc64le: [snip]/qemu/translate-all.c:175: tb_lock: Assertion 
>> `!have_tb_lock' failed.
>> Segmentation fault (core dumped)
>>
>> Debugging deeper found that its something to do with the variable type:
>>
>>     TCGv nb = tcg_temp_new();
>>     tcg_gen_andi_tl(nb, cpu_gpr[rB(ctx->opcode)], 0xFF);
>>     tcg_gen_brcondi_tl(TCG_COND_EQ, nb, 0, l1);
>>     [ Do something here]
>>     gen_set_label(l1);
>>     tcg_temp_free(nb);
>>
>> If I change the variable as "local temporary", the code works fine:
>>
>>     TCGv nb = tcg_temp_local_new();
>>     tcg_gen_andi_tl(nb, cpu_gpr[rB(ctx->opcode)], 0xFF);
>>     tcg_gen_brcondi_tl(TCG_COND_EQ, nb, 0, l1);
>>     [ Do something here]
>>     gen_set_label(l1);
>>     tcg_temp_free(nb);
>>
>> I see lot of code that is using temporaries for similar operations,
>> example target-ppc/translate.c:gen_check_align(). How is that working,
>> is this a bug there as well?
>
> You don't say what your "do something" code is doing, which
> is the critical question for whether you need a plain
> temporary or a local temporary. (See tcg/README.)

Lets bring full example here.

    TCGv nb = tcg_temp_new();
    tcg_gen_andi_tl(nb, cpu_gpr[rB(ctx->opcode)], 0xFF);
    tcg_gen_brcondi_tl(TCG_COND_EQ, nb, 0, l1);

    /* do something */
    gen_set_access_type(ctx, ACCESS_INT);
    EA = tcg_temp_new();
    gen_addr_register(ctx, EA);
    tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_LEQ);
    tcg_gen_addi_tl(EA, EA, 8);
    tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_LEQ);
    opc = tcg_const_i32(ctx->opcode);
    gen_helper_lxvl(cpu_env, opc, nb);      /* <--- That uses nb */
    tcg_temp_free_i32(opc);
    tcg_temp_free(EA);

    gen_set_label(l1);
    tcg_temp_free(nb);

> The plain temporary is only valid to the end of a basic
> block, and brcond ends a basic block. So you can free
> the temp after the brcond but you can't do anything
> else with it.

In the above case, assuming that nb is a plain temporary, case nb != 0
worked fine (by fluke?), i.e. no branch.

While when nb == 0, failed, i.e. branch taken to l1, and just free nb. I
am not using "nb" in this case.

> (This is what the PPC gen_check_align() does.)
> If you want to use 'nb' in the "do something" code then
> it must remain valid over the end of the basic block
> and you need a local temporary.

Understood, I need nb in that code, so I will use local temporary.

Regards
Nikunj