Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/7] target-ppc: Implement unsigned quadword left/right shift and unit tests

[joserz@linux.vnet.ibm.com]

On Mon, Dec 05, 2016 at 12:56:39PM +1100, David Gibson wrote:
> On Sat, Dec 03, 2016 at 05:37:27PM -0800, Richard Henderson wrote:
> > On 12/02/2016 09:00 PM, Jose Ricardo Ziviani wrote:
> > > +++ b/include/qemu/host-utils.h
> > > @@ -29,6 +29,33 @@
> > >  #include "qemu/bswap.h"
> > > 
> > >  #ifdef CONFIG_INT128
> > > +static inline void urshift(uint64_t *plow, uint64_t *phigh, uint32_t shift)
> > > +{
> > > +    __uint128_t val = ((__uint128_t)*phigh << 64) | *plow;
> > > +    val >>= (shift & 127);
> > > +    *phigh = val >> 64;
> > > +    *plow = val & 0xffffffffffffffff;
> > > +}
> > > +
> > > +static inline void ulshift(uint64_t *plow, uint64_t *phigh,
> > > +                           uint32_t shift, bool *overflow)
> > > +{
> > > +    __uint128_t val = ((__uint128_t)*phigh << 64) | *plow;
> > > +
> > > +    if (shift == 0) {
> > > +        return;
> > > +    }
> > > +
> > > +    if (shift > 127 || (val >> (128 - (shift & 127))) != 0) {
> > > +        *overflow = true;
> > > +    }
> > > +
> > > +    val <<= (shift & 127);
> > > +
> > > +    *phigh = val >> 64;
> > > +    *plow = val & 0xffffffffffffffff;
> > > +}
> > > +
> > 
> > This belongs in qemu/int128.h, not here.  And certainly not predicated on
> > CONFIG_INT128.
> 
> Is there actually any advantage to the __uint128_t based versions over
> the 64-bit versions?

Nothing special here. It just looks more clear (to me) to shift all 128
bits at once than 2x64. But I agree we won't loose to have just one
function outside CONFIG_INT128.

So, I'll remove these two functions and keep only the other two using
uint64_t types.

Anyway I get a bit confused about int128.h and host-utils.h. I see
functions like divu128 and divs128 that could be in int128.h, since
there is no similar operation in int128.h. Is there any rule about it?

Thank you guys for reviewing it!

> 
> -- 
> 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