[dpdk-dev] [PATCH] ixgbe: fix build with gcc 5

[dpdk-dev] [PATCH] ixgbe: fix build with gcc 5

[Panu Matilainen]

Add extra parenthesis to remove ambiguity on what we want to compare,
otherwise gcc 5 issues a "logical not is only applied to the left hand
side of comparison" warning which with -Werror fails the build.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
index 37e5bae..93a6a00 100644
--- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
+++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
@@ -2898,8 +2898,8 @@ STATIC s32 ixgbe_fc_autoneg_fiber(struct ixgbe_hw *hw)
 	 */
 
 	linkstat = IXGBE_READ_REG(hw, IXGBE_PCS1GLSTA);
-	if ((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE) == 0) ||
-	    (!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT) == 1)) {
+	if (((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE)) == 0) ||
+	    ((!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT)) == 1)) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING,
 			     "Auto-Negotiation did not complete or timed out");
 		goto out;
-- 
2.1.0

[dpdk-dev] [PATCH] i40e: fix build with gcc 5

[Panu Matilainen]

Eliminate embiguity in the condition which trips up a "logical not
is only applied to the left..." warning from gcc 5, causing build
failure with -Werror.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 lib/librte_pmd_i40e/i40e_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index c9f1026..ede5405 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -613,7 +613,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
 			     "rxq->nb_rx_desc=%d",
 			     rxq->rx_free_thresh, rxq->nb_rx_desc);
 		ret = -EINVAL;
-	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
+	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh == 0)) {
 		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
 			     "rxq->nb_rx_desc=%d, "
 			     "rxq->rx_free_thresh=%d",
-- 
2.1.0

Re: [dpdk-dev] [PATCH] i40e: fix build with gcc 5

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> Sent: Thursday, February 19, 2015 10:25 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] i40e: fix build with gcc 5
> 
> Eliminate embiguity in the condition which trips up a "logical not
> is only applied to the left..." warning from gcc 5, causing build
> failure with -Werror.
> 
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>  lib/librte_pmd_i40e/i40e_rxtx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
> index c9f1026..ede5405 100644
> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
> @@ -613,7 +613,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
>  			     "rxq->nb_rx_desc=%d",
>  			     rxq->rx_free_thresh, rxq->nb_rx_desc);
>  		ret = -EINVAL;
> -	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
> +	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh == 0)) {

Why just not:
else if (rxq->nb_rx_desc % rxq->rx_free_thresh != 0)
?

>  		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
>  			     "rxq->nb_rx_desc=%d, "
>  			     "rxq->rx_free_thresh=%d",
> --
> 2.1.0

Re: [dpdk-dev] [PATCH] i40e: fix build with gcc 5

[Panu Matilainen]

On 02/19/2015 01:05 PM, Ananyev, Konstantin wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
>> Sent: Thursday, February 19, 2015 10:25 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] i40e: fix build with gcc 5
>>
>> Eliminate embiguity in the condition which trips up a "logical not
>> is only applied to the left..." warning from gcc 5, causing build
>> failure with -Werror.
>>
>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>> ---
>>   lib/librte_pmd_i40e/i40e_rxtx.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
>> index c9f1026..ede5405 100644
>> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
>> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
>> @@ -613,7 +613,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
>>   			     "rxq->nb_rx_desc=%d",
>>   			     rxq->rx_free_thresh, rxq->nb_rx_desc);
>>   		ret = -EINVAL;
>> -	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
>> +	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh == 0)) {
>
> Why just not:
> else if (rxq->nb_rx_desc % rxq->rx_free_thresh != 0)
> ?

The same occurred to me right after hitting send, it'll make it a whole 
lot more obvious. I'll send another version.

	- Panu -

[dpdk-dev] [PATCH v2] i40e: fix build with gcc 5

[Panu Matilainen]

Eliminate ambiguity in the condition which trips up a "logical not
is only applied to the left..." warning from gcc 5, causing build
failure with -Werror. Besides non-ambiguous, the condition is
far more obvious this way.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 lib/librte_pmd_i40e/i40e_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index c9f1026..12c0831 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -613,7 +613,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
 			     "rxq->nb_rx_desc=%d",
 			     rxq->rx_free_thresh, rxq->nb_rx_desc);
 		ret = -EINVAL;
-	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
+	} else if (rxq->nb_rx_desc % rxq->rx_free_thresh != 0) {
 		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
 			     "rxq->nb_rx_desc=%d, "
 			     "rxq->rx_free_thresh=%d",
-- 
2.1.0

Re: [dpdk-dev] [PATCH v2] i40e: fix build with gcc 5

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> Sent: Thursday, February 19, 2015 11:21 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] i40e: fix build with gcc 5
> 
> Eliminate ambiguity in the condition which trips up a "logical not
> is only applied to the left..." warning from gcc 5, causing build
> failure with -Werror. Besides non-ambiguous, the condition is
> far more obvious this way.
> 
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>  lib/librte_pmd_i40e/i40e_rxtx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
> index c9f1026..12c0831 100644
> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
> @@ -613,7 +613,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
>  			     "rxq->nb_rx_desc=%d",
>  			     rxq->rx_free_thresh, rxq->nb_rx_desc);
>  		ret = -EINVAL;
> -	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
> +	} else if (rxq->nb_rx_desc % rxq->rx_free_thresh != 0) {
>  		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
>  			     "rxq->nb_rx_desc=%d, "
>  			     "rxq->rx_free_thresh=%d",
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.1.0

Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5

[Ananyev, Konstantin]

Hi Panu,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> Sent: Thursday, February 19, 2015 10:25 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5
> 
> Add extra parenthesis to remove ambiguity on what we want to compare,
> otherwise gcc 5 issues a "logical not is only applied to the left hand
> side of comparison" warning which with -Werror fails the build.
> 
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>  lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> index 37e5bae..93a6a00 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> @@ -2898,8 +2898,8 @@ STATIC s32 ixgbe_fc_autoneg_fiber(struct ixgbe_hw *hw)
>  	 */
> 
>  	linkstat = IXGBE_READ_REG(hw, IXGBE_PCS1GLSTA);
> -	if ((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE) == 0) ||
> -	    (!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT) == 1)) {
> +	if (((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE)) == 0) ||
> +	    ((!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT)) == 1)) {
>  		ERROR_REPORT1(IXGBE_ERROR_POLLING,
>  			     "Auto-Negotiation did not complete or timed out");
>  		goto out;

Unfortunately we are not supposed to change files under ixgbe subfirectory (except ixgbe_osdep.*).
Usually we deal with it just by:
If GCC_VERSION...
CFLAGS_ixgbe_common.o += -Wno...

You can have a look at lib/librte_pmd_ixgbe/Makefile, there are plenty of such things.
Konstantin


> --
> 2.1.0

Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5

[Panu Matilainen]

On 02/19/2015 02:02 PM, Ananyev, Konstantin wrote:
> Hi Panu,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
>> Sent: Thursday, February 19, 2015 10:25 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5
>>
>> Add extra parenthesis to remove ambiguity on what we want to compare,
>> otherwise gcc 5 issues a "logical not is only applied to the left hand
>> side of comparison" warning which with -Werror fails the build.
>>
>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>> ---
>>   lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
>> index 37e5bae..93a6a00 100644
>> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
>> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
>> @@ -2898,8 +2898,8 @@ STATIC s32 ixgbe_fc_autoneg_fiber(struct ixgbe_hw *hw)
>>   	 */
>>
>>   	linkstat = IXGBE_READ_REG(hw, IXGBE_PCS1GLSTA);
>> -	if ((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE) == 0) ||
>> -	    (!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT) == 1)) {
>> +	if (((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE)) == 0) ||
>> +	    ((!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT)) == 1)) {
>>   		ERROR_REPORT1(IXGBE_ERROR_POLLING,
>>   			     "Auto-Negotiation did not complete or timed out");
>>   		goto out;
>
> Unfortunately we are not supposed to change files under ixgbe subfirectory (except ixgbe_osdep.*).

Oh, sorry about that, I didn't realize there were untouchable files in 
the repo. Its not a very common setup :)

> Usually we deal with it just by:
> If GCC_VERSION...
> CFLAGS_ixgbe_common.o += -Wno...
>
> You can have a look at lib/librte_pmd_ixgbe/Makefile, there are plenty of such things.

Yup, noticed that but assumed the warning disablers were mainly for 
things that are not trivial to fix.

This one can be worked around just as easily with 
-Wlogical-not-parentheses, but since this flag is new to gcc 5 it can't 
really be added until gcc 5 is recognized as a supported version by the 
makefiles:
http://dpdk.org/dev/patchwork/patch/3452/

I'll send an updated version using warning disabler once other gcc-5 
support goes in.

	- Panu -

Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5

[Neil Horman]

On Thu, Feb 19, 2015 at 12:02:06PM +0000, Ananyev, Konstantin wrote:
> Hi Panu,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Panu Matilainen
> > Sent: Thursday, February 19, 2015 10:25 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 5
> > 
> > Add extra parenthesis to remove ambiguity on what we want to compare,
> > otherwise gcc 5 issues a "logical not is only applied to the left hand
> > side of comparison" warning which with -Werror fails the build.
> > 
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> > ---
> >  lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> > index 37e5bae..93a6a00 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c
> > @@ -2898,8 +2898,8 @@ STATIC s32 ixgbe_fc_autoneg_fiber(struct ixgbe_hw *hw)
> >  	 */
> > 
> >  	linkstat = IXGBE_READ_REG(hw, IXGBE_PCS1GLSTA);
> > -	if ((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE) == 0) ||
> > -	    (!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT) == 1)) {
> > +	if (((!!(linkstat & IXGBE_PCS1GLSTA_AN_COMPLETE)) == 0) ||
> > +	    ((!!(linkstat & IXGBE_PCS1GLSTA_AN_TIMED_OUT)) == 1)) {
> >  		ERROR_REPORT1(IXGBE_ERROR_POLLING,
> >  			     "Auto-Negotiation did not complete or timed out");
> >  		goto out;
> 
> Unfortunately we are not supposed to change files under ixgbe subfirectory (except ixgbe_osdep.*).
> Usually we deal with it just by:
> If GCC_VERSION...
> CFLAGS_ixgbe_common.o += -Wno...
> 
Why don't you just send a patch to the netdev list to fix ixgbe in the linux
tree, and then apply the same patch once it gets accepted.  Then the merge will
go smoothly when it comes down.  That would be much better than doing GCC
version ifdeffery.

Neil

> You can have a look at lib/librte_pmd_ixgbe/Makefile, there are plenty of such things.
> Konstantin
> 
> 
> > --
> > 2.1.0
> 
> 

Re: [dpdk-dev] [PATCH v2] i40e: fix build with gcc 5

[Thomas Monjalon]

> > Eliminate ambiguity in the condition which trips up a "logical not
> > is only applied to the left..." warning from gcc 5, causing build
> > failure with -Werror. Besides non-ambiguous, the condition is
> > far more obvious this way.
> > 
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks

[dpdk-dev] [PATCH v2] ixgbe: fix build with gcc 5

[Panu Matilainen]

gcc 5 supports a new logical-not-parentheses warning which
ixgbe_common.c triggers, causing build failure with -Werror.
Since this source must not be modified, silence the warning instead.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 lib/librte_pmd_ixgbe/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_pmd_ixgbe/Makefile b/lib/librte_pmd_ixgbe/Makefile
index 43870f7..9a5cd33 100644
--- a/lib/librte_pmd_ixgbe/Makefile
+++ b/lib/librte_pmd_ixgbe/Makefile
@@ -71,6 +71,10 @@ CFLAGS_ixgbe_common.o += -Wno-unused-but-set-variable
 CFLAGS_ixgbe_x550.o += -Wno-unused-but-set-variable -Wno-maybe-uninitialized
 endif
 
+ifeq ($(shell test $(GCC_VERSION) -ge 50 && echo 1), 1)
+CFLAGS_ixgbe_common.o += -Wno-logical-not-parentheses
+endif
+
 ifeq ($(shell test $(GCC_VERSION) -le 46 && echo 1), 1)
 CFLAGS_ixgbe_x550.o += -Wno-uninitialized
 CFLAGS_ixgbe_phy.o += -Wno-uninitialized
-- 
2.1.0

Re: [dpdk-dev] [PATCH v2] ixgbe: fix build with gcc 5

[Ananyev, Konstantin]

> -----Original Message-----
> From: Panu Matilainen [mailto:pmatilai@redhat.com]
> Sent: Tuesday, February 24, 2015 1:14 PM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin
> Subject: [PATCH v2] ixgbe: fix build with gcc 5
> 
> gcc 5 supports a new logical-not-parentheses warning which
> ixgbe_common.c triggers, causing build failure with -Werror.
> Since this source must not be modified, silence the warning instead.
> 
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> ---
>  lib/librte_pmd_ixgbe/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_pmd_ixgbe/Makefile b/lib/librte_pmd_ixgbe/Makefile
> index 43870f7..9a5cd33 100644
> --- a/lib/librte_pmd_ixgbe/Makefile
> +++ b/lib/librte_pmd_ixgbe/Makefile
> @@ -71,6 +71,10 @@ CFLAGS_ixgbe_common.o += -Wno-unused-but-set-variable
>  CFLAGS_ixgbe_x550.o += -Wno-unused-but-set-variable -Wno-maybe-uninitialized
>  endif
> 
> +ifeq ($(shell test $(GCC_VERSION) -ge 50 && echo 1), 1)
> +CFLAGS_ixgbe_common.o += -Wno-logical-not-parentheses
> +endif
> +
>  ifeq ($(shell test $(GCC_VERSION) -le 46 && echo 1), 1)
>  CFLAGS_ixgbe_x550.o += -Wno-uninitialized
>  CFLAGS_ixgbe_phy.o += -Wno-uninitialized
> --
> 2.1.0

Re: [dpdk-dev] [PATCH v2] ixgbe: fix build with gcc 5

[Thomas Monjalon]

> > gcc 5 supports a new logical-not-parentheses warning which
> > ixgbe_common.c triggers, causing build failure with -Werror.
> > Since this source must not be modified, silence the warning instead.
> > 
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks