Re: [Qemu-devel] [PATCH 20/40] Add vs{l,r} instructions.

[Aurelien Jarno]

On Wed, Jan 07, 2009 at 01:25:02PM -0800, Nathan Froyd wrote:
> On Mon, Jan 05, 2009 at 08:27:30PM +0100, Aurelien Jarno wrote:
> > On Mon, Jan 05, 2009 at 10:29:56AM -0800, Nathan Froyd wrote:
> > > FWIW, doing the check has the nice property of delivering the same
> > > results as real hardware.  If you're using a comparison program like
> > > ppctester, reducing those spurious failures is a win.  (There's already
> > > spurious failures for div instructions in corner cases.)
> > 
> > Then I am fine with that. Could you please just add a comment that it is
> > not necessary according to the spec, but done to match real hardware?
> 
> Done thusly.

Thanks, applied

> -Nathan
> 
> From aa19aa5e31aa9eec36a2cdfb2e1ed806555eda96 Mon Sep 17 00:00:00 2001
> From: Nathan Froyd <address@hidden>
> Date: Thu, 11 Dec 2008 12:02:03 -0800
> Subject: [PATCH 4/5] Add vs{l,r} instructions.
> 
> Add explanatory comment about why we check equality of shift counts.
> 
> Signed-off-by: Nathan Froyd <address@hidden>
> ---
>  target-ppc/helper.h    |    2 ++
>  target-ppc/op_helper.c |   39 +++++++++++++++++++++++++++++++++++++++
>  target-ppc/translate.c |    2 ++
>  3 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 009f516..e15d6b0 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -185,6 +185,8 @@ DEF_HELPER_3(vsubuws, void, avr, avr, avr)
>  DEF_HELPER_3(vrlb, void, avr, avr, avr)
>  DEF_HELPER_3(vrlh, void, avr, avr, avr)
>  DEF_HELPER_3(vrlw, void, avr, avr, avr)
> +DEF_HELPER_3(vsl, void, avr, avr, avr)
> +DEF_HELPER_3(vsr, void, avr, avr, avr)
>  DEF_HELPER_4(vsldoi, void, avr, avr, avr, i32)
>  DEF_HELPER_3(vspltb, void, avr, avr, i32)
>  DEF_HELPER_3(vsplth, void, avr, avr, i32)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index ef19bde..81abd63 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -2491,6 +2491,45 @@ void helper_vsel (ppc_avr_t *r, ppc_avr_t *a, 
> ppc_avr_t *b, ppc_avr_t *c)
>      r->u64[1] = (a->u64[1] & ~c->u64[1]) | (b->u64[1] & c->u64[1]);
>  }
>  
> +#if defined(WORDS_BIGENDIAN)
> +#define LEFT 0
> +#define RIGHT 1
> +#else
> +#define LEFT 1
> +#define RIGHT 0
> +#endif
> +/* The specification says that the results are undefined if all of the
> + * shift counts are not identical.  We check to make sure that they are
> + * to conform to what real hardware appears to do.  */
> +#define VSHIFT(suffix, leftp)                                           \
> +    void helper_vs##suffix (ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)   \
> +    {                                                                   \
> +        int shift = b->u8[LO_IDX*0x15] & 0x7;                           \
> +        int doit = 1;                                                   \
> +        int i;                                                          \
> +        for (i = 0; i < ARRAY_SIZE(r->u8); i++) {                       \
> +            doit = doit && ((b->u8[i] & 0x7) == shift);                 \
> +        }                                                               \
> +        if (doit) {                                                     \
> +            if (shift == 0) {                                           \
> +                *r = *a;                                                \
> +            } else if (leftp) {                                         \
> +                uint64_t carry = a->u64[LO_IDX] >> (64 - shift);        \
> +                r->u64[HI_IDX] = (a->u64[HI_IDX] << shift) | carry;     \
> +                r->u64[LO_IDX] = a->u64[LO_IDX] << shift;               \
> +            } else {                                                    \
> +                uint64_t carry = a->u64[HI_IDX] << (64 - shift);        \
> +                r->u64[LO_IDX] = (a->u64[LO_IDX] >> shift) | carry;     \
> +                r->u64[HI_IDX] = a->u64[HI_IDX] >> shift;               \
> +            }                                                           \
> +        }                                                               \
> +    }
> +VSHIFT(l, LEFT)
> +VSHIFT(r, RIGHT)
> +#undef VSHIFT
> +#undef LEFT
> +#undef RIGHT
> +
>  #define VSL(suffix, element)                                            \
>      void helper_vsl##suffix (ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)  \
>      {                                                                   \
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index e92156f..65a80f2 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6356,6 +6356,8 @@ GEN_VXFORM(vsubsws, 0, 30);
>  GEN_VXFORM(vrlb, 2, 0);
>  GEN_VXFORM(vrlh, 2, 1);
>  GEN_VXFORM(vrlw, 2, 2);
> +GEN_VXFORM(vsl, 2, 7);
> +GEN_VXFORM(vsr, 2, 11);
>  GEN_VXFORM(vpkuhum, 7, 0);
>  GEN_VXFORM(vpkuwum, 7, 1);
>  GEN_VXFORM(vpkuhus, 7, 2);
> -- 
> 1.6.0.5
> 
> 
> 
> 

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