[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm |
Date: |
Wed, 4 Dec 2013 00:32:23 +0000 |
On 4 December 2013 00:10, Richard Henderson <address@hidden> wrote:
> On 12/04/2013 10:51 AM, Peter Maydell wrote:
>> @@ -184,6 +184,18 @@ static TCGv_i64 cpu_reg(DisasContext *s, int reg)
>> }
>> }
>>
>> +/* read a cpu register in 32bit/64bit mode to dst */
>> +static void read_cpu_reg(DisasContext *s, TCGv_i64 dst, int reg, int sf)
>> +{
>> + if (reg == 31) {
>> + tcg_gen_movi_i64(dst, 0);
>> + } else if (sf) {
>> + tcg_gen_mov_i64(dst, cpu_X[reg]);
>> + } else { /* (!sf) */
>> + tcg_gen_ext32u_i64(dst, cpu_X[reg]);
>> + }
>> +}
>
> I think this should be more like cpu_reg and return a TCGv instead of force a
> copy into a newly generated temporary. You've got a pool of auto-freeing
> temporaries, after all.
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'm still a bit on the fence about that pool of auto-freeing temporaries.
Manual temp management is certainly a fertile source of decoder bugs,
but in the longer term we might want to push the functionality down into
the common code rather than having an ad-hoc thing in one decoder.)
>> + if (op) { /* CBNZ */
>> + tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
>> + } else { /* CBZ */
>> + tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
>> + }
>
> Similar comments to TBNZ/TBZ apply here.
Agreed.
thanks
-- PMM
- Re: [Qemu-devel] [PATCH 03/12] target-arm: A64: provide functions for accessing FPCR and FPSR, (continued)
[Qemu-devel] [PATCH 08/12] target-arm: A64: add support for B and BL insns, Peter Maydell, 2013/12/03
[Qemu-devel] [PATCH 09/12] target-arm: A64: add support for BR, BLR and RET insns, Peter Maydell, 2013/12/03
[Qemu-devel] [PATCH 07/12] target-arm: A64: expand decoding skeleton for system instructions, Peter Maydell, 2013/12/03