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

[Eric Blake]

On 01/09/2017 08:10 PM, Jose Ricardo Ziviani wrote:
> Implements 128-bit left shift and right shift as well as their
> testcases. By design, shift silently mods by 128, so the caller is
> responsible to assert the shift range if necessary.
> 
> Left shift sets the overflow flag if any non-zero digit is shifted out.
> 
> Examples:
>  ulshift(&low, &high, 250, &overflow);
>  equivalent: n << 122
> 
>  urshift(&low, &high, -2);
>  equivalent: n << 126
> 
> Signed-off-by: Jose Ricardo Ziviani <address@hidden>
> ---

> +typedef struct {
> +    uint64_t low;
> +    uint64_t high;
> +    uint64_t rlow;
> +    uint64_t rhigh;
> +    int32_t shift;
> +    bool overflow;
> +} test_data;
> +
> +static const test_data test_ltable[] = {
> +    { 0x4C7ULL, 0x0ULL, 0x00000000000004C7ULL,
> +      0x0000000000000000ULL,   0, false },

I might have laid it out as:

{ 0x00000000000004c7ULL, 0x0000000000000000ULL,
  0x00000000000004c7ULL, 0x0000000000000000ULL,
  0, false }

to make the pre- and post-shift values line up better.  It's not fatal
to the patch, so it's up to the maintainer if they want a v6 to improve
the alignment.

> +    { 0x8888888888888888ULL, 0x9999999999999999ULL,
> +      0x8000000000000000ULL, 0x9888888888888888ULL, 60, true },
> +    { 0x8888888888888888ULL, 0x9999999999999999ULL,
> +      0x0000000000000000ULL, 0x8888888888888888ULL, 64, true },

These two are the most legible.

> +};
> +
> +static const test_data test_rtable[] = {

> +++ b/util/host-utils.c
> @@ -161,3 +161,67 @@ int divs128(int64_t *plow, int64_t *phigh, int64_t 
> divisor)
>  }
>  #endif
>  
> +/**
> + * urshift - 128-bit Unsigned Right Shift.
> + * @plow: in/out - lower 64-bit integer.
> + * @phigh: in/out - higher 64-bit integer.
> + * @shift: in - bytes to shift, between 0 and 127.
> + *
> + * Result is zero-extended and stored in plow/phigh, which are
> + * input/output variables. Shift values outside the range will
> + * be mod to 128. In other words, the caller is responsible to
> + * verify/assert both the shift range and plow/phigh pointers.
> + */

Duplicating docs in the .h and .c doesn't hurt, but risks one getting
out of date; we have other spots that put the docs in the .h (where
callers will look up what's available) or the .c (where the
implementation is there to check against the docs). I don't have any
strong preference on how to do it, though, so I don't mind leaving it as is.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature