[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] target-mips: Implement Loongson Multimedia I
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v2] target-mips: Implement Loongson Multimedia Instructions |
Date: |
Mon, 17 Sep 2012 21:03:46 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Sep 17, 2012 at 11:54:59AM -0700, Richard Henderson wrote:
> 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.
Yes, I agree it is the same once sign extended, I agree. What I meant is
that for the same input values you might have an exception on ADD and
not on DADD.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
address@hidden http://www.aurel32.net