Re: [dpdk-dev] [PATCH v8 2/3] eal: add u64 bit variant for reciprocal

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/26/2018 5:04 AM, Pavan Nikhilesh wrote:
> Currently, rte_reciprocal only supports unsigned 32bit divisors. This
> commit adds support for unsigned 64bit divisors.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>

<...>

> +static uint64_t
> +divide_128_div_64_to_64(uint64_t u1, uint64_t u0, uint64_t v, uint64_t *r)
> +{
> +	const uint64_t b = (1ULL << 32); /* Number base (16 bits). */
> +	uint64_t un1, un0,           /* Norm. dividend LSD's. */
> +		 vn1, vn0,           /* Norm. divisor digits. */
> +		 q1, q0,             /* Quotient digits. */
> +		 un64, un21, un10,   /* Dividend digit pairs. */
> +		 rhat;               /* A remainder. */
> +	int s;                       /* Shift amount for norm. */
> +
> +	/* If overflow, set rem. to an impossible value. */
> +	if (u1 >= v) {
> +		if (r != NULL)
> +			*r = (uint64_t) -1;
> +		return (uint64_t) -1;
> +	}
> +
> +	/* Count leading zeros. */
> +	s = __builtin_clzll(v);
> +	if (s > 0) {
> +		v = v << s;
> +		un64 = (u1 << s) | ((u0 >> (64 - s)) & (-s >> 31));

Hi Pavan,

This is causing a cppcheck warning [1]. s is `int` and we know here s > 0,
so `-s >> 31` should be 0xffffffff right, what is the point of this operation
instead of using hardcoded value?

[1]
(error) Shifting signed 32-bit value by 31 bits is undefined behaviour