Re: [PATCH v5 1/3] ixgbe: Cleanups

[Vlad Zolotarov <vladz-RmZWMc9puTNJc61us3aD9laTQe2KTcn/@public.gmane.org>]

On 03/09/15 13:29, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: Wodkowski, PawelX
>> Sent: Monday, March 09, 2015 11:09 AM
>> To: Ananyev, Konstantin; Vlad Zolotarov; dev-VfR2kkLFssw@public.gmane.org
>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>
>> On 2015-03-09 11:49, Ananyev, Konstantin wrote:
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Vlad Zolotarov
>>>> Sent: Monday, March 09, 2015 10:21 AM
>>>> To: dev-VfR2kkLFssw@public.gmane.org
>>>> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
>>>>
>>>>      - Removed the not needed casting.
>>>>      - ixgbe_dev_rx_init(): shorten the lines by defining a local alias variable to access
>>>>                             &dev->data->dev_conf.rxmode.
>>>>
>>>> Signed-off-by: Vlad Zolotarov <vladz-RmZWMc9puTNJc61us3aD9laTQe2KTcn/@public.gmane.org>
>>>> ---
>>>>    lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ++++++++++++---------------
>>>>    1 file changed, 12 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> index 72c65df..609b5fd 100644
>>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>>> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq)
>>>>    	int diag, i;
>>>>
>>>>    	/* allocate buffers in bulk directly into the S/W ring */
>>>> -	alloc_idx = (uint16_t)(rxq->rx_free_trigger -
>>>> -				(rxq->rx_free_thresh - 1));
>>>> +	alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1);
>>> I think all these extra casts came in to keep icc 12.* compiling without warnings.
>>> I am agree that they are unnecessary.
>>> Though if we still have to support icc 12.* we either need to keep them, or find
>>> some other way to keep it happy.
>>> Konstantin
>>>
>> What warnings icc 12.* is throwing?
> Try and see :)
>
>> Only warning I can think of here is
>> signed -> unsigned implicit cast.
> If I remember things correctly, it considered result at right side of '=' operator as unsigned int,
> and then complained that we assign it to smaller size (unsigned short) operand.

If that's the case - that's a clear compiler bug.

>
>> Changing '1' to '1U' helps?
> Don't think so, but you are welcome to try.
>
> Konstantin
>
>>
>> --
>> Pawel