On 4 December 2013 00:10, Richard Henderson <rth@xxxxxxxxxxx> 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
