| Message ID | 1425896433-12452-2-git-send-email-vladz@cloudius-systems.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 795A69A91; Mon, 9 Mar 2015 11:20:40 +0100 (CET) Received: from mail-we0-f170.google.com (mail-we0-f170.google.com [74.125.82.170]) by dpdk.org (Postfix) with ESMTP id 36B5A5A9C for <dev@dpdk.org>; Mon, 9 Mar 2015 11:20:38 +0100 (CET) Received: by wevl61 with SMTP id l61so8058760wev.10 for <dev@dpdk.org>; Mon, 09 Mar 2015 03:20:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=nFZOfS82JHyYcWcYBTc6Z9fvG2amIUDtlOoUQH6izhA=; b=eeZFoCw48fLop/F8HNyQC4qTxI95elFOZ1DdXOgDaIqkQtMmhSL/PQJf6UV5ijNbVN 2807yiFRDcik8kQLkOkN4kCFi7Fbx+DcgSHra/xczLTIhdeO5Df1tULv/NgTtD/fdop6 2GOTRaujm1evMRh0g3FqzVE+cnHzurqPXhsWAGim1tikOkfeW/O0DeLPlfhljUgPnosu fGwzWtoFZ8BfS00hQFH2HIhJ3P7z3/2x0lBvDTS955KU8Gn2Y2COlKMLA8jC6OPm0NHt 7ts4tmfYocuHv6fz/FTw/Zll/H/0S88mZDigHkq1jdEv47Z/vmIxXUI1hdcz2rUzBKTT Jf4Q== X-Gm-Message-State: ALoCoQlI6TOgp7in4b7UbAcaoSzlwSNZQxZp1IVg5gYHodX1Obq5XWpZoH5KD/WU7PIykVbVHUGJ X-Received: by 10.194.61.12 with SMTP id l12mr55947644wjr.139.1425896438002; Mon, 09 Mar 2015 03:20:38 -0700 (PDT) Received: from vladz-laptop.localdomain ([109.65.117.109]) by mx.google.com with ESMTPSA id md2sm18728782wic.19.2015.03.09.03.20.36 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Mar 2015 03:20:37 -0700 (PDT) From: Vlad Zolotarov <vladz@cloudius-systems.com> To: dev@dpdk.org Date: Mon, 9 Mar 2015 12:20:31 +0200 Message-Id: <1425896433-12452-2-git-send-email-vladz@cloudius-systems.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1425896433-12452-1-git-send-email-vladz@cloudius-systems.com> References: <1425896433-12452-1-git-send-email-vladz@cloudius-systems.com> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Vladislav Zolotarov
March 9, 2015, 10:20 a.m. UTC
- 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(-)
Comments
> -----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 > rxep = &rxq->sw_ring[alloc_idx]; > diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep, > rxq->rx_free_thresh); > @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq) > IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger); > > /* update state of internal queue structure */ > - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger + > - rxq->rx_free_thresh); > + rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; > if (rxq->rx_free_trigger >= rxq->nb_rx_desc) > - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1); > + rxq->rx_free_trigger = rxq->rx_free_thresh - 1; > > /* no errors */ > return 0; > @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > uint32_t rxcsum; > uint16_t buf_size; > uint16_t i; > + struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode; > > PMD_INIT_FUNC_TRACE(); > hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > * Configure CRC stripping, if any. > */ > hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0); > - if (dev->data->dev_conf.rxmode.hw_strip_crc) > + if (rx_conf->hw_strip_crc) > hlreg0 |= IXGBE_HLREG0_RXCRCSTRP; > else > hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP; > @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > /* > * Configure jumbo frame support, if any. > */ > - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) { > + if (rx_conf->jumbo_frame == 1) { > hlreg0 |= IXGBE_HLREG0_JUMBOEN; > maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS); > maxfrs &= 0x0000FFFF; > - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16); > + maxfrs |= (rx_conf->max_rx_pkt_len << 16); > IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs); > } else > hlreg0 &= ~IXGBE_HLREG0_JUMBOEN; > @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > * Reset crc_len in case it was changed after queue setup by a > * call to configure. > */ > - rxq->crc_len = (uint8_t) > - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 : > - ETHER_CRC_LEN); > + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN; > > /* Setup the Base and Length of the Rx Descriptor Rings */ > bus_addr = rxq->rx_ring_phys_addr; > @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > /* > * Configure Header Split > */ > - if (dev->data->dev_conf.rxmode.header_split) { > + if (rx_conf->header_split) { > if (hw->mac.type == ixgbe_mac_82599EB) { > /* Must setup the PSRTYPE register */ > uint32_t psrtype; > @@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > IXGBE_PSRTYPE_IPV6HDR; > IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype); > } > - srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size << > + srrctl = ((rx_conf->split_hdr_size << > IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) & > IXGBE_SRRCTL_BSIZEHDR_MASK); > srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS; > @@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > */ > rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM); > rxcsum |= IXGBE_RXCSUM_PCSD; > - if (dev->data->dev_conf.rxmode.hw_ip_checksum) > + if (rx_conf->hw_ip_checksum) > rxcsum |= IXGBE_RXCSUM_IPPCSE; > else > rxcsum &= ~IXGBE_RXCSUM_IPPCSE; > @@ -3709,7 +3706,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) > if (hw->mac.type == ixgbe_mac_82599EB || > hw->mac.type == ixgbe_mac_X540) { > rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL); > - if (dev->data->dev_conf.rxmode.hw_strip_crc) > + if (rx_conf->hw_strip_crc) > rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP; > else > rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP; > -- > 2.1.0
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? Only warning I can think of here is signed -> unsigned implicit cast. Changing '1' to '1U' helps?
> -----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. >Changing '1' to '1U' helps? Don't think so, but you are welcome to try. Konstantin > > > -- > Pawel
On 03/09/15 12: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. Fix icc maybe? I'm sorry, but what do I miss here? Both alloc_idx, rxq->rx_free_trigger and rxq->rx_free_thresh are uint16_t So, according to C standard the result is also uint16_t thus no casting is needed: the result of a subtraction generating a negative number in an unsigned type is well-defined: 1. [...] A computation involving unsigned operands can never overflow, because a result that cannot be represented by the resulting unsigned integer type is reduced modulo the number that is one greater than the largest value that can be represented by the resulting type. (ISO/IEC 9899:1999 (E) §6.2.5/9) Could u, pls., be more specific and send here the error generated by icc after my patches? thanks, vlad > Konstantin > >> rxep = &rxq->sw_ring[alloc_idx]; >> diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep, >> rxq->rx_free_thresh); >> @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq) >> IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger); >> >> /* update state of internal queue structure */ >> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger + >> - rxq->rx_free_thresh); >> + rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; >> if (rxq->rx_free_trigger >= rxq->nb_rx_desc) >> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1); >> + rxq->rx_free_trigger = rxq->rx_free_thresh - 1; >> >> /* no errors */ >> return 0; >> @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> uint32_t rxcsum; >> uint16_t buf_size; >> uint16_t i; >> + struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode; >> >> PMD_INIT_FUNC_TRACE(); >> hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); >> @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> * Configure CRC stripping, if any. >> */ >> hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0); >> - if (dev->data->dev_conf.rxmode.hw_strip_crc) >> + if (rx_conf->hw_strip_crc) >> hlreg0 |= IXGBE_HLREG0_RXCRCSTRP; >> else >> hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP; >> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> /* >> * Configure jumbo frame support, if any. >> */ >> - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) { >> + if (rx_conf->jumbo_frame == 1) { >> hlreg0 |= IXGBE_HLREG0_JUMBOEN; >> maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS); >> maxfrs &= 0x0000FFFF; >> - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16); >> + maxfrs |= (rx_conf->max_rx_pkt_len << 16); >> IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs); >> } else >> hlreg0 &= ~IXGBE_HLREG0_JUMBOEN; >> @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> * Reset crc_len in case it was changed after queue setup by a >> * call to configure. >> */ >> - rxq->crc_len = (uint8_t) >> - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 : >> - ETHER_CRC_LEN); >> + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN; >> >> /* Setup the Base and Length of the Rx Descriptor Rings */ >> bus_addr = rxq->rx_ring_phys_addr; >> @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> /* >> * Configure Header Split >> */ >> - if (dev->data->dev_conf.rxmode.header_split) { >> + if (rx_conf->header_split) { >> if (hw->mac.type == ixgbe_mac_82599EB) { >> /* Must setup the PSRTYPE register */ >> uint32_t psrtype; >> @@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> IXGBE_PSRTYPE_IPV6HDR; >> IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype); >> } >> - srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size << >> + srrctl = ((rx_conf->split_hdr_size << >> IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) & >> IXGBE_SRRCTL_BSIZEHDR_MASK); >> srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS; >> @@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> */ >> rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM); >> rxcsum |= IXGBE_RXCSUM_PCSD; >> - if (dev->data->dev_conf.rxmode.hw_ip_checksum) >> + if (rx_conf->hw_ip_checksum) >> rxcsum |= IXGBE_RXCSUM_IPPCSE; >> else >> rxcsum &= ~IXGBE_RXCSUM_IPPCSE; >> @@ -3709,7 +3706,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> if (hw->mac.type == ixgbe_mac_82599EB || >> hw->mac.type == ixgbe_mac_X540) { >> rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL); >> - if (dev->data->dev_conf.rxmode.hw_strip_crc) >> + if (rx_conf->hw_strip_crc) >> rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP; >> else >> rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP; >> -- >> 2.1.0
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. > >> Changing '1' to '1U' helps? > Don't think so, but you are welcome to try. > > Konstantin > >> >> -- >> Pawel
> -----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. > > > > >> Changing '1' to '1U' helps? > > Don't think so, but you are welcome to try. > > > > Konstantin > > > >> > >> -- > >> Pawel
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 > ,
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov > Sent: Monday, March 9, 2015 5:14 PM > To: Ananyev, Konstantin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" > <konstantin.ananyev@intel.com> > wrote: > > > > 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 Hi, For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): $ make T=x86_64-native-linuxapp-icc install CC=icc Build complete $ git log --pretty=format:"%h - %an: %s" -4 b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... $ icc --version icc (ICC) 13.1.1 20130313 John. --
On Mar 9, 2015 8:01 PM, "Mcnamara, John" <john.mcnamara@intel.com> wrote: > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov > > Sent: Monday, March 9, 2015 5:14 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > > > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" > > <konstantin.ananyev@intel.com> > > wrote: > > > > > 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 > > Hi, > > For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): > > $ make T=x86_64-native-linuxapp-icc install CC=icc > Build complete > > $ git log --pretty=format:"%h - %an: %s" -4 > b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups > 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... > 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... > 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... > > $ icc --version > icc (ICC) 13.1.1 20130313 That's worth a lot to me!.. :D > > John. > -- > >
On Mar 9, 2015 8:01 PM, "Mcnamara, John" <john.mcnamara@intel.com> wrote: > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov > > Sent: Monday, March 9, 2015 5:14 PM > > To: Ananyev, Konstantin > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > > > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" > > <konstantin.ananyev@intel.com> > > wrote: > > > > > 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 > > Hi, > > For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): > > $ make T=x86_64-native-linuxapp-icc install CC=icc > Build complete > > $ git log --pretty=format:"%h - %an: %s" -4 > b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups > 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... > 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... > 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... > > $ icc --version > icc (ICC) 13.1.1 20130313 Thanks a lot, John. > > John. > -- > >
> > From: Vladislav Zolotarov [mailto:vladz@cloudius-systems.com] > Sent: Monday, March 09, 2015 6:22 PM > To: Mcnamara, John > Cc: dev@dpdk.org; Ananyev, Konstantin > Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > > On Mar 9, 2015 8:01 PM, "Mcnamara, John" <john.mcnamara@intel.com> wrote: > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov > > > Sent: Monday, March 9, 2015 5:14 PM > > > To: Ananyev, Konstantin > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > > > > > On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" > > > <konstantin.ananyev@intel.com> > > > wrote: > > > > > > 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 > > > > Hi, > > > > For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): > > > > $ make T=x86_64-native-linuxapp-icc install CC=icc > > Build complete > > > > $ git log --pretty=format:"%h - %an: %s" -4 > > b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups > > 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... > > 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... > > 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... > > > > $ icc --version > > icc (ICC) 13.1.1 20130313 > Thanks a lot, John. As I said my worry was about 12.*. icc v13* and above, are not that picky. After applying the patch, indeed all these lines make icc 12.1 to generate warnings. See below for details. Though with icc 12.1 current dpdk.org main branch will generate ~2K unhandled warnings all over the places anyway... Which makes me think that probably DPDK support for icc 12.* was scrapped already, I just didn't notice that. Konstantin $ icc -v icc version 12.1.0 (gcc version 4.5.0 compatibility) $ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread -march=native -DRTE_MACHINE_CPU FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPU FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHI NE_CPUFLAG_AVX -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_ CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG _AVX -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include -include /lo cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 -Wall -w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 15527 -o ixgbe_rxtx.o -c /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c .... /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose signif icant bits alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1); ^ /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose significant bits rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; ^ /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose significant bits rxq->rx_free_trigger = rxq->rx_free_thresh - 1; ....
On 03/09/15 21:13, Ananyev, Konstantin wrote: > >> From: Vladislav Zolotarov [mailto:vladz@cloudius-systems.com] >> Sent: Monday, March 09, 2015 6:22 PM >> To: Mcnamara, John >> Cc: dev@dpdk.org; Ananyev, Konstantin >> Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups >> >> >> On Mar 9, 2015 8:01 PM, "Mcnamara, John" <john.mcnamara@intel.com> wrote: >>>> -----Original Message----- >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov >>>> Sent: Monday, March 9, 2015 5:14 PM >>>> To: Ananyev, Konstantin >>>> Cc: dev@dpdk.org >>>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups >>>> >>>> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" >>>> <konstantin.ananyev@intel.com> >>>> wrote: >>>>>>> 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 >>> Hi, >>> >>> For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): >>> >>> $ make T=x86_64-native-linuxapp-icc install CC=icc >>> Build complete >>> >>> $ git log --pretty=format:"%h - %an: %s" -4 >>> b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups >>> 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... >>> 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... >>> 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... >>> >>> $ icc --version >>> icc (ICC) 13.1.1 20130313 >> Thanks a lot, John. > As I said my worry was about 12.*. > icc v13* and above, are not that picky. > After applying the patch, indeed all these lines make icc 12.1 to generate warnings. > See below for details. > Though with icc 12.1 current dpdk.org main branch will generate ~2K unhandled warnings all over the places anyway... > Which makes me think that probably DPDK support for icc 12.* was scrapped already, I just didn't notice that. So, I suppose this concludes this specific discussion, right? ;) > Konstantin > > $ icc -v > icc version 12.1.0 (gcc version 4.5.0 compatibility) > > $ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread -march=native -DRTE_MACHINE_CPU > FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPU > FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHI > NE_CPUFLAG_AVX -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_ > CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG > _AVX -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include -include /lo > cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 -Wall > -w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 15527 -o ixgbe_rxtx.o -c /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c > > .... > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark #2259: > non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose signif > icant bits > alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1); > ^ > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose significant bits > rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; > ^ > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark #2259: non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose significant bits > rxq->rx_free_trigger = rxq->rx_free_thresh - 1; > > .... >
> -----Original Message----- > From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] > Sent: Monday, March 09, 2015 7:32 PM > To: Ananyev, Konstantin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > > > > On 03/09/15 21:13, Ananyev, Konstantin wrote: > > > >> From: Vladislav Zolotarov [mailto:vladz@cloudius-systems.com] > >> Sent: Monday, March 09, 2015 6:22 PM > >> To: Mcnamara, John > >> Cc: dev@dpdk.org; Ananyev, Konstantin > >> Subject: RE: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > >> > >> > >> On Mar 9, 2015 8:01 PM, "Mcnamara, John" <john.mcnamara@intel.com> wrote: > >>>> -----Original Message----- > >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vladislav Zolotarov > >>>> Sent: Monday, March 9, 2015 5:14 PM > >>>> To: Ananyev, Konstantin > >>>> Cc: dev@dpdk.org > >>>> Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups > >>>> > >>>> On Mar 9, 2015 6:39 PM, "Ananyev, Konstantin" > >>>> <konstantin.ananyev@intel.com> > >>>> wrote: > >>>>>>> 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 > >>> Hi, > >>> > >>> For what it is worth there aren't any warnings with ICC 13 and the 1/3 patch (+ the previous patchset): > >>> > >>> $ make T=x86_64-native-linuxapp-icc install CC=icc > >>> Build complete > >>> > >>> $ git log --pretty=format:"%h - %an: %s" -4 > >>> b5d06e4 - Vladislav Zolotarov: ixgbe: Cleanups > >>> 3cd0367 - Vladislav Zolotarov: ixgbe: Unify the rx_pkt_bulk callback ... > >>> 10ed30e - Vladislav Zolotarov: ixgbe: Bug fix: Properly configure Rx ... > >>> 2a5bc6a - Vladislav Zolotarov: ixgbe: Use the rte_le_to_cpu_xx()/rte_... > >>> > >>> $ icc --version > >>> icc (ICC) 13.1.1 20130313 > >> Thanks a lot, John. > > As I said my worry was about 12.*. > > icc v13* and above, are not that picky. > > After applying the patch, indeed all these lines make icc 12.1 to generate warnings. > > See below for details. > > Though with icc 12.1 current dpdk.org main branch will generate ~2K unhandled warnings all over the places anyway... > > Which makes me think that probably DPDK support for icc 12.* was scrapped already, I just didn't notice that. > > So, I suppose this concludes this specific discussion, right? ;) I believe so. Sorry for the noise generated. Konstantin > > > Konstantin > > > > $ icc -v > > icc version 12.1.0 (gcc version 4.5.0 compatibility) > > > > $ icc -Wp,-MD,./.ixgbe_rxtx.o.d.tmp -m64 -pthread -march=native -DRTE_MACHINE_CPU > > FLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPU > > FLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHI > > NE_CPUFLAG_AVX -DRTE_COMPILE_TIME_CPUFLAGS=RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_ > > CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG > > _AVX -I/local/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include -include /lo > > cal/kananye1/dpdk.org/x86_64-native-linuxapp-icc/include/rte_config.h -O3 -Wall > > -w2 -diag-disable 271 -diag-warning 1478 -diag-disable 13368 -diag-disable 15527 -o ixgbe_rxtx.o -c > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c > > > > .... > > > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1035): remark #2259: > > non-pointer conversion from "int" to "uint16_t={unsigned short}" may lose signif > > icant bits > > alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1); > > ^ > > > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1064): remark #2259: non-pointer conversion from "int" to > "uint16_t={unsigned short}" may lose significant bits > > rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; > > ^ > > > > /local/kananye1/dpdk.org/lib/librte_pmd_ixgbe/ixgbe_rxtx.c(1067): remark #2259: non-pointer conversion from "int" to > "uint16_t={unsigned short}" may lose significant bits > > rxq->rx_free_trigger = rxq->rx_free_thresh - 1; > > > > .... > >
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); rxep = &rxq->sw_ring[alloc_idx]; diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep, rxq->rx_free_thresh); @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq) IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger); /* update state of internal queue structure */ - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger + - rxq->rx_free_thresh); + rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; if (rxq->rx_free_trigger >= rxq->nb_rx_desc) - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1); + rxq->rx_free_trigger = rxq->rx_free_thresh - 1; /* no errors */ return 0; @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) uint32_t rxcsum; uint16_t buf_size; uint16_t i; + struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode; PMD_INIT_FUNC_TRACE(); hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) * Configure CRC stripping, if any. */ hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0); - if (dev->data->dev_conf.rxmode.hw_strip_crc) + if (rx_conf->hw_strip_crc) hlreg0 |= IXGBE_HLREG0_RXCRCSTRP; else hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP; @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) /* * Configure jumbo frame support, if any. */ - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) { + if (rx_conf->jumbo_frame == 1) { hlreg0 |= IXGBE_HLREG0_JUMBOEN; maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS); maxfrs &= 0x0000FFFF; - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16); + maxfrs |= (rx_conf->max_rx_pkt_len << 16); IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs); } else hlreg0 &= ~IXGBE_HLREG0_JUMBOEN; @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) * Reset crc_len in case it was changed after queue setup by a * call to configure. */ - rxq->crc_len = (uint8_t) - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 : - ETHER_CRC_LEN); + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN; /* Setup the Base and Length of the Rx Descriptor Rings */ bus_addr = rxq->rx_ring_phys_addr; @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) /* * Configure Header Split */ - if (dev->data->dev_conf.rxmode.header_split) { + if (rx_conf->header_split) { if (hw->mac.type == ixgbe_mac_82599EB) { /* Must setup the PSRTYPE register */ uint32_t psrtype; @@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) IXGBE_PSRTYPE_IPV6HDR; IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype); } - srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size << + srrctl = ((rx_conf->split_hdr_size << IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) & IXGBE_SRRCTL_BSIZEHDR_MASK); srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS; @@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) */ rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM); rxcsum |= IXGBE_RXCSUM_PCSD; - if (dev->data->dev_conf.rxmode.hw_ip_checksum) + if (rx_conf->hw_ip_checksum) rxcsum |= IXGBE_RXCSUM_IPPCSE; else rxcsum &= ~IXGBE_RXCSUM_IPPCSE; @@ -3709,7 +3706,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) if (hw->mac.type == ixgbe_mac_82599EB || hw->mac.type == ixgbe_mac_X540) { rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL); - if (dev->data->dev_conf.rxmode.hw_strip_crc) + if (rx_conf->hw_strip_crc) rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP; else rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP;