From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDgTJ-0002sw-5f for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:55:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TDgTH-0003Qb-49 for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:55:05 -0400 Received: from mail-qa0-f52.google.com ([209.85.216.52]:56664) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TDgTG-0003OP-Vu for qemu-devel@nongnu.org; Mon, 17 Sep 2012 14:55:03 -0400 Received: by qabg14 with SMTP id g14so1795760qab.4 for ; Mon, 17 Sep 2012 11:55:02 -0700 (PDT) Sender: Richard Henderson Message-ID: <50577203.2070300@twiddle.net> Date: Mon, 17 Sep 2012 11:54:59 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1333127593-7841-1-git-send-email-rth@twiddle.net> <20120908003216.GL6791@ohm.aurel32.net> In-Reply-To: <20120908003216.GL6791@ohm.aurel32.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] target-mips: Implement Loongson Multimedia Instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org, proljc@gmail.com 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~