From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42906)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <proljc@gmail.com>) id 1SDanu-0007xj-EQ
	for qemu-devel@nongnu.org; Fri, 30 Mar 2012 08:19:48 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <proljc@gmail.com>) id 1SDann-00086o-1E
	for qemu-devel@nongnu.org; Fri, 30 Mar 2012 08:19:42 -0400
Received: from mail-lpp01m010-f45.google.com ([209.85.215.45]:43116)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <proljc@gmail.com>) id 1SDanm-00086b-Ns
	for qemu-devel@nongnu.org; Fri, 30 Mar 2012 08:19:34 -0400
Received: by lahe6 with SMTP id e6so706353lah.4
	for <qemu-devel@nongnu.org>; Fri, 30 Mar 2012 05:19:32 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <4F75936F.8030302@twiddle.net>
References: <1333077432-22228-1-git-send-email-proljc@gmail.com>
	<1333077432-22228-9-git-send-email-proljc@gmail.com>
	<4F75936F.8030302@twiddle.net>
Date: Fri, 30 Mar 2012 20:19:32 +0800
Message-ID: <CAJBMM-v82Lo1-hv6GGcH270op-2NqQXYpFNFb36f3m376ZKFcA@mail.gmail.com>
From: Jia Liu <proljc@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH V4 08/11] Add MIPS DSP Bit/Manipulation
	instructions Support
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org, aurelien@aurel32.net

On Fri, Mar 30, 2012 at 7:05 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 03/29/2012 11:17 PM, Jia Liu wrote:
>> + =A0 =A0int32_t temp;
>> + =A0 =A0uint32_t rd;
>> + =A0 =A0int i, last;
>> +
>> + =A0 =A0temp =3D rt & MIPSDSP_LO;
>> + =A0 =A0rd =3D 0;
>> + =A0 =A0for (i =3D 0; i < 16; i++) {
>> + =A0 =A0 =A0 =A0last =3D temp % 2;
>> + =A0 =A0 =A0 =A0temp =3D temp >> 1;
>
> temp should not be signed, as that % doesn't do what you wanted.
>
>
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0imm3 =3D tcg_const_i32(imm);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0imm2 =3D tcg_const_i32(imm);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0imm1 =3D tcg_const_i32(imm);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0imm0 =3D tcg_const_i32(imm);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(imm3, imm3, 24=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(imm2, imm2, 16=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(imm1, imm1, 8)=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], imm=
3, imm2);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], cpu=
_gpr[rd], imm1);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], cpu=
_gpr[rd], imm0);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_temp_free(imm3);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_temp_free(imm2);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_temp_free(imm1);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_temp_free(imm0);
>
> Err, this is an *immediate*.
>
> =A0imm =3D (ctx->opcode >> 16) & 0xFF;
> =A0tcg_gen_movi(cpu_gpr[rd], imm * 0x01010101);
>
>
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rt3 =3D tcg_const_i32(0);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rt2 =3D tcg_const_i32(0);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rt1 =3D tcg_const_i32(0);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rt0 =3D tcg_const_i32(0);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_andi_i32(rt3, cpu_gpr[r=
t], 0xFF);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_andi_i32(rt2, cpu_gpr[r=
t], 0xFF);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_andi_i32(rt1, cpu_gpr[r=
t], 0xFF);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_andi_i32(rt0, cpu_gpr[r=
t], 0xFF);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(rt3, rt3, 24);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(rt2, rt2, 16);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_shli_i32(rt1, rt1, 8);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], rt3=
, rt2);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], cpu=
_gpr[rd], rt1);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_gen_or_i32(cpu_gpr[rd], cpu=
_gpr[rd], rt0);
>
> I hadn't been asking for you to inline replv, only repl.
> But if you want to do this, at least only compute t=3Drt&0xff once.
> That said, I suspect the * 0x01010101 trick is fairly efficient on
> most hosts these days.
>
>
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0TCGv temp_rt =3D tcg_const_i32(=
rt);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gen_helper_insv(cpu_gpr[rt], cp=
u_env,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0cpu_gpr[rs], cpu_gpr[rt]);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tcg_temp_free(temp_rt);
>
> temp_rt is unused.
>

Thank you very much for pointing the errors!
I'll fix them.

>
>
> r~
>

Regards,
Jia.