On 09/07/2012 05:32 PM, Aurelien Jarno wrote:
>> + do_shift:
>> + switch (opc) {
>> + case OPC_SLL_CP2:
>> + case OPC_DSLL_CP2:
>> + tcg_gen_shl_i64(t0, t0, t1);
>> + break;
>> + case OPC_SRA_CP2:
>> + case OPC_DSRA_CP2:
>> + /* Since SRA is UndefinedResult without sign-extended inputs,
>> + we can treat SRA and DSRA the same. */
>> + tcg_gen_sar_i64(t0, t0, t1);
>> + break;
>> + case OPC_SRL_CP2:
>> + /* We want to shift in zeros for SRL; zero-extend first. */
>> + tcg_gen_ext32u_i64(t0, t0);
>> + /* FALLTHRU */
>> + case OPC_DSRL_CP2:
>> + tcg_gen_shr_i64(t0, t0, t1);
>> + break;
>> + }
>
> You probably want to and t1 with 0x3f, to make sure to not have a shift
> larger then 64.
Done. Though as discussed elsewhere today I think we ought to make this
merely undefined results as opposed to undefined behaviour in TCG.
>> + /* Shifts larger than MAX produce zero. */
>> + tcg_gen_setcondi_i64(TCG_COND_LTU, t1, t1, shift_max);
>> + tcg_gen_neg_i64(t1, t1);
>
> I guess you want tcg_gen_subi_i64(t1, t1, 1);
No. You're confusing the computations of
(x >= 32) - 1
and
-(x < 32)
Logically the same results but the computation is different.
And the later will of course be smaller on i386 due to neg insn.
>> + case OPC_ADD_CP2:
>> + case OPC_DADD_CP2:
>> + {
>> + /* Since ADD is UndefinedResult without sign-extended inputs,
>> + we can treat both ADD and DADD the same. */
>
> I don't think this is correct. For ADD, the result has to be signed
> extended. Also the exception condition is not the same for ADD and DADD.
Fixed the sign extension here and SUB.
The exception condition *is* the same, after the sign extension is done.
Please go back and compare the code in the existing ADD/DADD expansion.
r~
