Re: [Qemu-devel] [PATCH V3 09/12] Add helper functions for MIPS DSP Compare-Pick instructions

[Jia Liu]

On Wed, Mar 28, 2012 at 1:42 AM, Richard Henderson <address@hidden> wrote:
> On 03/27/12 02:24, Jia Liu wrote:
>> +    uint32_t cc3 = 0, cc2 = 0, cc1 = 0, cc0 = 0;
>> +    uint32_t flag;
> ...
>> +    if (rs3 == rt3)
>> +        cc3 = 1;
>
> Clearer as "cc3 = (rs3 == rt3);", etc.
>

I'll try to make code more clearer.

> I begin to wonder if it wouldn't be better to model the DSPControl
> word as individual variables of their respective fields.  That way
> you don't need to involve ENV so much.
>
> E.g.
>
>  Assume cpu_dspcc is a TCGv_i32 variable containing 4 bits.
>
>  case CMPGU_COND_QB:
>      gen_helper_cmpdu_cond_qb(cpu_gr[rd], cpu_gr[rs], cpu_gr[rt]);
>      break;
>  case CMPGDU_COND_QB:
>      gen_helper_cmpdu_cond_qb(cpu_dspcc, cpu_gr[rs], cpu_gr[rt]);
>      tcg_gen_mov(cpu_gr[rd], cpu_dspcc);
>      break;
>
> No need for two separate helpers.  Even if you don't decide to split
> the field, you probably don't want to duplicate so much code in the
> helpers file.
>

Thanks, I'll combine them.

>
> r~

Regards,
Jia.