Hi Petar, On Sun, Oct 28, 2012 at 6:58 AM, Jovanovic, Petar <pet...@mips.com> 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.