Re: [Qemu-devel] [PATCH v2 5/6] target-ppc: add vprtyb[w/d/q] instructions

[David Gibson]

On Thu, Oct 27, 2016 at 07:16:24AM -0700, Richard Henderson wrote:
> On 10/27/2016 01:36 AM, Nikunj A Dadhania wrote:
> > Right, it does reduce number of operations:
> > 
> > +#define SIZE_MASK(x) ((1ULL << (x)) - 1)
> > +static uint64_t vparity(uint64_t f1, uint64_t f2, int size)
> > +{
> > +    uint64_t res = f1 ^ f2;
> > +    if (size == 8) return res;
> > +    return vparity(res & SIZE_MASK(size/2), res >> (size/2), size/2);
> > +}
> 
> Why are you using recursion for something that should be 5 operations?
> You're making this more complicated than it needs to be.
> 
>   uint64_t res = b->u64[0] ^ b->u64[1];
>   res ^= res >> 32;
>   res ^= res >> 16;
>   res ^= res >> 8;
>   r->u64[LO_IDX] = res & 1;

We do need to implement it at multiple sizes, which makes it a bit
more complex.  But I wonder if it makes sense to do this without a
helper, something like

gen_vprty()
{
    ...
    gen(shift right 8)
    gen(xor)
    if (size > 2)
       gen(shift right 16)
       gen(xor)
    if (size > 4)
       gen(shift right 32)
       gen(xor)
    if (size > 8)
       gen(xor hi and low words)
    gen(mask result bits)
}

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature