Date: Mon, 15 Dec 2008 23:11:52 +0100
From: Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH 01/42] target-ppc: add Altivec logical
	operations.
Message-ID: <20081215221152.GE8523@volta.aurel32.net>
References: <1229307315-16807-1-git-send-email-froydnj@codesourcery.com>
	<1229307315-16807-2-git-send-email-froydnj@codesourcery.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <1229307315-16807-2-git-send-email-froydnj@codesourcery.com>
Reply-To: qemu-devel@nongnu.org
To: Nathan Froyd <froydnj@codesourcery.com>
Cc: qemu-devel@nongnu.org

On Sun, Dec 14, 2008 at 06:14:34PM -0800, Nathan Froyd wrote:
> 
> Signed-off-by: Nathan Froyd <froydnj@codesourcery.com>
> ---
>  target-ppc/translate.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 4c4f9ef..70047c7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6109,6 +6109,24 @@ GEN_VR_STX(svx, 0x07, 0x07);
>  /* As we don't emulate the cache, stvxl is stricly equivalent to stvx */
>  GEN_VR_STX(svxl, 0x07, 0x0F);
>  
> +/* Logical operations */
> +#define GEN_VX_LOGICAL(name, tcg_op, xo)                                \
> +GEN_HANDLER(name, 0x04, (xo >> 1) & 0x1f, (xo >> 6) & 0x1f, 0x00000000, PPC_ALTIVEC) \
> +{                                                                       \
> +    if (unlikely(!ctx->altivec_enabled)) {                              \
> +        gen_exception(ctx, POWERPC_EXCP_VPU);                           \
> +        return;                                                         \
> +    }                                                                   \
> +    tcg_op(cpu_avrh[rD(ctx->opcode)], cpu_avrh[rA(ctx->opcode)], cpu_avrh[rB(ctx->opcode)]); \
> +    tcg_op(cpu_avrl[rD(ctx->opcode)], cpu_avrl[rA(ctx->opcode)], cpu_avrl[rB(ctx->opcode)]); \
> +}
> +
> +GEN_VX_LOGICAL(vand, tcg_gen_and_i64, 1028);
> +GEN_VX_LOGICAL(vandc, tcg_gen_andc_i64, 1092);
> +GEN_VX_LOGICAL(vor, tcg_gen_or_i64, 1156);
> +GEN_VX_LOGICAL(vxor, tcg_gen_xor_i64, 1220);
> +GEN_VX_LOGICAL(vnor, tcg_gen_nor_i64, 1284);

I know those decimal value comes from the PowerPC manual, but the whole
QEMU code uses hexadecimal values instead. Also it is usually passed
directly as opc2 and opc3 values. I guess it is better to continue like
that for consistencies.

Otherwise the patch looks good, I'll apply it when that is fixed.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net