On 10/24/2016 02:14 AM, Nikunj A Dadhania wrote:
> +#define EXTRACT_BITS(size) \
> +static inline uint##size##_t extract_bits_u##size(uint##size##_t reg, \
> + uint##size##_t start, \
> + uint##size##_t end) \
> +{ \
> + uint##size##_t nr_mask_bits = end - start + 1; \
> + uint##size##_t val = 1; \
> + uint##size##_t mask = (val << nr_mask_bits) - 1; \
> + uint##size##_t shifted_reg = reg >> ((size - 1) - end); \
> + return shifted_reg & mask; \
> +}
> +
> +EXTRACT_BITS(64);
> +EXTRACT_BITS(32);
We already have extract32 and extract64, which you're (nearly) duplicating.
> +#define MASK(size, max_val) \
> +static inline uint##size##_t mask_u##size(uint##size##_t start, \
> + uint##size##_t end) \
> +{ \
> + uint##size##_t ret, max_bit = size - 1; \
> + \
> + if (likely(start == 0)) { \
> + ret = max_val << (max_bit - end); \
> + } else if (likely(end == max_bit)) { \
> + ret = max_val >> start; \
> + } else { \
> + ret = (((uint##size##_t)(-1ULL)) >> (start)) ^ \
> + (((uint##size##_t)(-1ULL) >> (end)) >> 1); \
> + if (unlikely(start > end)) { \
> + return ~ret; \
> + } \
> + } \
Why the two likely cases? Doesn't the third case cover them?
Also, (uint##size##_t)(-1ULL) should be just (uint##size##_t)-1.
Please remove all the other unnecessarry parenthesis too.
Hmph. I see you've copied all this silliness from translate.c, so...
nevermind, I guess. Let's leave this a near-exact copy.
> +#define LEFT_ROTATE(size) \
> +static inline uint##size##_t left_rotate_u##size(uint##size##_t val, \
> + uint##size##_t shift) \
> +{ \
> + if (!shift) { \
> + return val; \
> + } \
> + \
> + uint##size##_t left_val = extract_bits_u##size(val, 0, shift - 1); \
> + uint##size##_t right_val = val & mask_u##size(shift, size - 1); \
> + \
> + return right_val << shift | left_val; \
> +}
> +
> +LEFT_ROTATE(32);
> +LEFT_ROTATE(64);
We already have rol32 and rol64.
Which I see are broken for shift == 0. Let's please fix that, as a separate
patch, like so:
return (word << shift) | (word >> ((32 - shift) & 31));
r~
