From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v8 2/3] eal: add u64 bit variant for
	reciprocal
Date: Sun, 28 Oct 2018 01:06:10 +0100
Message-ID: <7ce0fa65-01ff-4fc3-7714-df0711a0361b@intel.com>
References: <1504032378-5483-1-git-send-email-pbhagavatula@caviumnetworks.com>
 <20180126050451.5953-1-pbhagavatula@caviumnetworks.com>
 <20180126050451.5953-2-pbhagavatula@caviumnetworks.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: dev@dpdk.org
To: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>, thomas@monjalon.net,
 cristian.dumitrescu@intel.com, stephen@networkplumber.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga12.intel.com (mga12.intel.com [192.55.52.136])
 by dpdk.org (Postfix) with ESMTP id 694AF201
 for <dev@dpdk.org>; Sun, 28 Oct 2018 02:06:15 +0200 (CEST)
In-Reply-To: <20180126050451.5953-2-pbhagavatula@caviumnetworks.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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