From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:52016)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ULyD2-0007Ks-Hs
	for qemu-devel@nongnu.org; Sat, 30 Mar 2013 12:00:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ULyCz-0000H4-4m
	for qemu-devel@nongnu.org; Sat, 30 Mar 2013 12:00:48 -0400
Received: from mail-da0-x236.google.com ([2607:f8b0:400e:c00::236]:54578)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ULyCy-0000Gx-IU
	for qemu-devel@nongnu.org; Sat, 30 Mar 2013 12:00:45 -0400
Received: by mail-da0-f54.google.com with SMTP id p1so553397dad.27
	for <qemu-devel@nongnu.org>; Sat, 30 Mar 2013 09:00:43 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <51570C28.7010706@twiddle.net>
Date: Sat, 30 Mar 2013 09:00:40 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <1364657589-16123-1-git-send-email-aurelien@aurel32.net>
In-Reply-To: <1364657589-16123-1-git-send-email-aurelien@aurel32.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/2] tcg-ia64: implement mulu2_i32/i64 and
 muls2_i32/i64
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: Aurelien Jarno <aurelien@aurel32.net>
Cc: qemu-devel@nongnu.org

On 03/30/2013 08:33 AM, Aurelien Jarno wrote:
> +    if (const_arg1 && arg1 != 0) {
> +        opc2 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5,
> +                          TCG_REG_R2, arg1, TCG_REG_R0);
> +    } else if (is_signed) {
> +        opc2 = tcg_opc_i29(TCG_REG_P0, OPC_SXT4_I29, TCG_REG_R2, arg1);
> +    } else {
> +        opc2 = tcg_opc_i29(TCG_REG_P0, OPC_ZXT4_I29, TCG_REG_R2, arg1);
> +    }
> +    if (const_arg2 && arg2 != 0) {
> +        opc3 = tcg_opc_a5(TCG_REG_P0, OPC_ADDL_A5,
> +                          TCG_REG_R3, arg2, TCG_REG_R0);
> +    } else if (is_signed) {
> +        opc3 = tcg_opc_i29(TCG_REG_P0, OPC_SXT4_I29, TCG_REG_R3, arg2);
> +    } else {
> +        opc3 = tcg_opc_i29(TCG_REG_P0, OPC_ZXT4_I29, TCG_REG_R3, arg2);
> +    }
> +

Why the check form arg[12] != 0?  Seems like a wasted test, especially if
the optimizer is enabled and we ought never see it.

I would have said you shouldn't bother implementing the _i32 versions for
ia64, because the generic fallback would produce the same code.  But in
this case it's more about doing the job with fewer bundles.

Otherwise, the code looks correct,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~