Hi Richard Peter Jovanovic and Aurelien,
On Wed, Oct 31, 2012 at 1:26 PM, Richard Henderson <r...@twiddle.net> wrote:
> On 2012-10-31 01:44, Peter Maydell wrote:
>> On 30 October 2012 15:34, Jia Liu <pro...@gmail.com> wrote:
>>> On Mon, Oct 29, 2012 at 9:40 PM, Jovanovic, Petar <pet...@mips.com> wrote:
>>>>> imm = (int16_t)(imm << 6) >> 6;
>>>>
>>>> result of a bitwise shift of a signed type and a negative vlaue is
>>>> implementation-defined, so you can not rely on that.
>>>>
>>>
>>> I think it will take a 10bits signed value sign extend into 16bits
>>> signed value, and I've tested it with negative values, it working
>>> well.
>>
>> You cannot rely on the behaviour of a specific compiler implementation
>> as evidence that a piece of code is correct. C has a standard which
>> defines what is and is not valid.
>
> Indeed. The only portable way is
>
> val = ((val & (sign | (sign - 1))) ^ sign) - sign
>
> with all unsigned types, and "sign" set to the sign bit.
>
Thank you very much for the code.
Is this time OK?
static void gen_mipsdsp_bitinsn(CPUMIPSState *env, DisasContext *ctx,
uint32_t op1, uint32_t op2,
int ret, int val)
{
const char *opn = "mipsdsp Bit/ Manipulation";
TCGv t0;
TCGv val_t;
if (ret == 0) {
/* Treat as NOP. */
MIPS_DEBUG("NOP");
return;
}
t0 = tcg_temp_new();
val_t = tcg_temp_new();
gen_load_gpr(val_t, val);
#define SIGN_EXTEND10_16(val, sign) \
val = (((val & (sign | (sign - 1))) ^ sign) - sign)
switch (op1) {
case OPC_ABSQ_S_PH_DSP:
switch (op2) {
case OPC_BITREV:
check_dsp(ctx);
gen_helper_bitrev(cpu_gpr[ret], val_t);
break;
case OPC_REPL_QB:
check_dsp(ctx);
{
uint32_t imm;
target_long result;
imm = (ctx->opcode >> 16) & 0xFF;
result = imm << 24 | imm << 16 | imm << 8 | imm;
result = (int32_t)result;
tcg_gen_movi_tl(cpu_gpr[ret], result);
}
break;
case OPC_REPLV_QB:
check_dsp(ctx);
tcg_gen_ext8u_tl(cpu_gpr[ret], val_t);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 8);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 16);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_ext32s_tl(cpu_gpr[ret], cpu_gpr[ret]);
break;
case OPC_REPL_PH:
check_dsp(ctx);
{
uint16_t imm;
imm = (ctx->opcode >> 16) & 0x03FF;
SIGN_EXTEND10_16(imm, 0x200);
tcg_gen_movi_tl(cpu_gpr[ret],
(target_long)(int32_t)
((uint32_t)imm << 16 | imm));
}
break;
case OPC_REPLV_PH:
check_dsp(ctx);
tcg_gen_ext16u_tl(cpu_gpr[ret], val_t);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 16);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_ext32s_tl(cpu_gpr[ret], cpu_gpr[ret]);
break;
}
break;
#ifdef TARGET_MIPS64
case OPC_ABSQ_S_QH_DSP:
switch (op2) {
case OPC_REPL_OB:
check_dsp(ctx);
{
target_ulong imm;
imm = (ctx->opcode >> 16) & 0xFF;
imm = (imm << 8) | imm;
imm = (imm << 16) | imm;
imm = (imm << 32) | imm;
tcg_gen_movi_tl(cpu_gpr[ret], imm);
break;
}
case OPC_REPL_PW:
check_dsp(ctx);
{
target_long imm;
imm = (ctx->opcode >> 16) & 0x03FF;
SIGN_EXTEND10_16(imm, 0x200);
tcg_gen_movi_tl(cpu_gpr[ret],
(imm << 32) | (imm & 0xFFFFFFFF));
break;
}
case OPC_REPL_QH:
check_dsp(ctx);
{
target_ulong imm;
imm = (ctx->opcode >> 16) & 0x03FF;
SIGN_EXTEND10_16(imm, 0x200);
imm = imm & 0xFFFF;
imm = (imm << 48) | (imm << 32) | (imm << 16) | imm;
tcg_gen_movi_tl(cpu_gpr[ret], imm);
break;
}
case OPC_REPLV_OB:
check_dsp(ctx);
tcg_gen_ext8u_tl(cpu_gpr[ret], val_t);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 8);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 16);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 32);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
break;
case OPC_REPLV_PW:
check_dsp(ctx);
tcg_gen_ext32u_i64(cpu_gpr[ret], val_t);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 32);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
break;
case OPC_REPLV_QH:
check_dsp(ctx);
tcg_gen_ext16u_tl(cpu_gpr[ret], val_t);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 16);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
tcg_gen_shli_tl(t0, cpu_gpr[ret], 32);
tcg_gen_or_tl(cpu_gpr[ret], cpu_gpr[ret], t0);
break;
}
break;
#endif
}
#undef SIGN_EXTEND10_16
tcg_temp_free(t0);
tcg_temp_free(val_t);
(void)opn; /* avoid a compiler warning */
MIPS_DEBUG("%s", opn);
}
>>
>> Having said that, right shift of negative signed integers is one of
>> those bits of implementation defined behaviour which we allow ourselves
>> to rely on in QEMU because all the platforms we care about behave
>> that way. (That integers are 2s complement representation is another.)
>
> Also very true. I don't like seeing the code in question though.
> We've several implementations of sign-extend-to-N-bits functions
> throughout qemu; we ought to unify them.
>
>
> r~
Regards,
Jia.
