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

[Peter Maydell]

On 4 December 2013 00:48, Richard Henderson <address@hidden> wrote:
> On 12/04/2013 01:32 PM, Peter Maydell wrote:
>> You're right that we can just make this function return the TCGv
>> temp rather than making the caller pass one in. Are you suggesting
>> the 64-bit case should return cpu_X[reg] rather than a copy of it,
>> though? I think it would be pretty hard to reason about if you had to
>> remember that sometimes you got a trashable copy and sometimes
>> the real register.
>
> I think that the normal case is to write to the output in one step, and thus
> having an input overlap an output isn't your problem but tcg's.  I would think
> the case of multi-step output to be fairly rare, and easily solvable by always
> allocating an output temporary.

So I had a look at this, and it seems to make code like this (from
the handling for logic ops with shifted registers):

    tcg_rm = read_cpu_reg(s, rm, sf);

    if (shift_amount) {
        shift_reg_imm(tcg_rm, tcg_rm, sf,
                      shift_type, shift_amount);
    }

    if (invert) {
        tcg_gen_not_i64(tcg_rm, tcg_rm);
        /* we zero extend later on (!sf) */
    }

a bit awkward if the value returned from read_cpu_reg() isn't a
trashable copy, because of the sequence of "maybe we need
to change the value like this" conditionals. The code basically
needs to copy the return value from read_cpu_reg() into its
own temporary immediately. There's a similar thing in the
conditional-select code where the else-case's value might be
inverted and might be incremented and might be neither.

I think that kind of thing tips the balance in favour of having
read_cpu_reg() always return a trashable temporary.

thanks
-- PMM