Re: [Qemu-devel] [PATCH v12 09/14] target-mips: Add ASE DSP bit/manipulation instructions

[Jia Liu]

Hi Petar,

On Sun, Oct 28, 2012 at 6:58 AM, Jovanovic, Petar <address@hidden> wrote:
> +        case OPC_REPL_PH:
> +            check_dsp(ctx);
> +            {
> +                imm = (ctx->opcode >> 16) & 0x03FF;
> +                tcg_gen_movi_tl(cpu_gpr[ret], \
> +                                (target_long)((int32_t)imm << 16 | \
> +                                (uint32_t)(uint16_t)imm));
> +            }
>
> 10-bit integer in REPL.PH is signed, so this code will not work for negative
> values.
> You need to sign-extend it, e.g. something like this:
>
> +                imm = (ctx->opcode >> 16) & 0x03FF;
> +                if (imm & (1 << 9)) {
> +                  /* imm is negative, sign-extend it to 16 bits. */
> +                  imm |= 0xFC00;
> +                }
> +                tcg_gen_movi_tl(cpu_gpr[ret], \
> +                                (target_long)((int32_t)imm << 16 | \
> +                                (uint32_t)(uint16_t)imm));
>
> As far as I can see, the test cases for REPL.PH in
> tests/tcg/mips/mips32-dsp/repl_ph.c cover only positive values.
> Make sure you include test cases for negative values as well.
>
> Petar

New code:
        case OPC_REPL_PH:
            check_dsp(ctx);
            {
                imm = (ctx->opcode >> 16) & 0x03FF;
                imm = (int16_t)(imm << 6) >> 6;
                tcg_gen_movi_tl(cpu_gpr[ret], \
                                (target_long)(int32_t)((int32_t)imm << 16 | \
                                (uint32_t)(uint16_t)imm));
            }
            break;

And the new test:
#include<stdio.h>
#include<assert.h>

int main()
{
    int rd, result;

    result = 0x01BF01BF;
    __asm
        ("repl.ph %0, 0x1BF\n\t"
         : "=r"(rd)
        );
    assert(rd == result);

    result = 0xFFFFFFFF;
    __asm
        ("repl.ph %0, -1\n\t"
         : "=r"(rd)
        );
    assert(rd == result);

    return 0;
}

is it OK?
And the other tests have be fixed.

Regards,
Jia.