Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups

[Vladislav Zolotarov <vladz@cloudius-systems.com>]

On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
wrote:
>
>
>
> > -----Original Message-----
> > From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> > Sent: Monday, March 09, 2015 3:58 PM
> > To: Ananyev, Konstantin; Wodkowski, PawelX; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
> >
> >
> >
> > 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@dpdk.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@dpdk.org] On Behalf Of Vlad Zolotarov
> > >>>> Sent: Monday, March 09, 2015 10:21 AM
> > >>>> To: dev@dpdk.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@cloudius-systems.com>
> > >>>> ---
> > >>>>    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.
>
> Might be, though if we still have to support it, there is no much choice
I am afraid.

Well to begin with could anybody who has this icc thing (preferably the
latest version) post the compilation errors u are talking about. Let's
continue this discussion with some specific things in hands. So far there
were a lot of "maybe"s and "as far as i remember"s. Could u, guys, pls., be
more specific so that i could address the real issues and not just your
fears? ;)

Thanks,
Vlad

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