Re: [Qemu-devel] [PATCH V3 08/12] Add helper functions for MIPS DSP Bit/Manipulation instructions

[Richard Henderson]

On 03/27/12 02:24, Jia Liu wrote:
> +uint32_t helper_bitrev(uint32_t rt)
> +{
> +    int32_t temp;
> +    uint32_t rd;
> +
> +    temp = rt & MIPSDSP_LO;
> +    rd = temp;
> +
> +    return rd;
> +}

Forgot to actually reverse the bits.

> +uint32_t helper_repl_qb(uint32_t imm)
...
> +uint32_t helper_repl_ph(uint32_t imm)

Should not be helpers.  The full 32-bit immediate can be
computed directly in the translator.

> +void helper_insv(int reg_rt, uint32_t rs, uint32_t rt)
> +{
> +    uint32_t pos, size, msb, lsb, rs_f, rt_f;
> +    uint32_t temp, temprs, temprt;
> +    target_ulong dspc;
> +
> +    dspc = env->active_tc.DSPControl;
> +    pos  = dspc & 0x1F;
> +    size = (dspc >> 7) & 0x1F;
> +    msb  = pos + size - 1;
> +    lsb  = pos;
> +
> +    if (lsb > msb)
> +        return;
> +
> +    rs_f = (((int32_t)0x01 << (msb - lsb + 1 + 1)) - 1) << lsb;
> +    rt_f = rs_f ^ 0xFFFFFFFF;
> +    temprs = rs & rs_f;
> +    temprt = rt & rt_f;
> +    temp = temprs | temprt;
> +    env->active_tc.gpr[reg_rt] = temp;
> +}

Err... why does this modify the env directly, rather than
return a value?  "return rt" would seem to be the correct
way to leave the value of reg_rt unchanged...


r~