Re: [Qemu-devel] [PATCH v3 34/34] target-alpha: Emulate LL/SC using cmpxchg helpers

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> On 09/15/2016 07:38 AM, Alex Bennée wrote:
>>> +    lab_fail = gen_new_label();
>>> > +    lab_done = gen_new_label();
>>> > +    tcg_gen_brcond_i64(TCG_COND_NE, addr, cpu_lock_addr, lab_fail);
>>> > +    tcg_temp_free_i64(addr);
>>> >
>>> > -        lab_fail = gen_new_label();
>>> > -        lab_done = gen_new_label();
>>> > -        tcg_gen_brcond_i64(TCG_COND_NE, addr, cpu_lock_addr, lab_fail);
>>> > +    val = tcg_temp_new_i64();
>>> > +    tcg_gen_atomic_cmpxchg_i64(val, cpu_lock_addr, cpu_lock_value,
>>> > +                               load_gpr(ctx, ra), mem_idx, op);
>>> > +    free_context_temps(ctx);
>> I don't quite follow what free_context_temps() is doing and why it needs
>> to be done twice during the building of this instructions TCGOps?
>>
>
> It's releasing the zero, sink, imm TCGv temporaries from DisasContext, as
> they're going out of scope within each basic block.  Since they're freed,
> they'll be re-created as necessary in the next basic block.
>
> The two (unlikely) relevant events are: (1) address register = zero register,
> so the zero temp becomes invalid, and (2) dest register = zero register, so 
> the
> zero/sink temps become invalid.

Ahh ok. The ARM code just allocates temps on demand including for its
zero register which I guess means multiple ones could be assigned. There
is also temp allocation logic to free them at the end of the block.

I wonder if this is something that should be more of a feature of the
core TCG code because let's face it temp tracking is a pain.

>
> It's the kind of thing that really shouldn't ever happen for non-malicious
> code, but you don't want qemu to abort when invalid temps are used.  Perhaps a
> comment is warranted here...

Does the code generator suffer if temps aren't freed as soon as
possible?  Is there a reason why we don't just do a general free all
temps at the end of the block?

--
Alex Bennée