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

[Richard Henderson]

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 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.


r~