[v2] net/iavf: fix compilation warnings

Message ID 20230609090236.70859-1-zhichaox.zeng@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/iavf: fix compilation warnings |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation warning apply patch failure
ci/Intel-compilation warning apply issues

Commit Message

Zhichao Zeng June 9, 2023, 9:02 a.m. UTC
Use fallthrough comments to ignore compilation warnings instead of
diagnostic pragmas, which are not recognized by old GCC(below GCC7).

Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on SSE")
Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on AVX2")
Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on AVX512")
Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
---
 drivers/net/iavf/iavf_rxtx_vec_avx2.c   | 11 ++++++++---
 drivers/net/iavf/iavf_rxtx_vec_avx512.c | 11 ++++++++---
 drivers/net/iavf/iavf_rxtx_vec_sse.c    |  7 ++++---
 3 files changed, 20 insertions(+), 9 deletions(-)
  

Comments

Gao, DaxueX June 12, 2023, 8:32 a.m. UTC | #1
> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: 2023年6月9日 17:03
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Gao, DaxueX
> <daxuex.gao@intel.com>; Zeng, ZhichaoX <zhichaox.zeng@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: [PATCH v2] net/iavf: fix compilation warnings
> 
> Use fallthrough comments to ignore compilation warnings instead of diagnostic
> pragmas, which are not recognized by old GCC(below GCC7).
> 
> Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on SSE")
> Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on AVX2")
> Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on AVX512")
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
Tested-by: Daxue Gao <daxuex.gao@intel.com>
  
Thomas Monjalon June 12, 2023, 9:01 a.m. UTC | #2
12/06/2023 10:32, Gao, DaxueX:
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> > Use fallthrough comments to ignore compilation warnings instead of diagnostic
> > pragmas, which are not recognized by old GCC(below GCC7).
> > 
> > Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on SSE")
> > Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on AVX2")
> > Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on AVX512")
> > Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> Tested-by: Daxue Gao <daxuex.gao@intel.com>

Please squash the fix in the original commits above,
so we won't have any compilation issue when doing "git bisect".

In the meantime, I am pulling the branch next-net-intel
without the Rx timestamp support for SSE/AVX2/AVX512.
  
Qi Zhang June 12, 2023, 12:29 p.m. UTC | #3
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, June 12, 2023 5:02 PM
> To: Zeng, ZhichaoX <zhichaox.zeng@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>; Gao, DaxueX <daxuex.gao@intel.com>;
> david.marchand@redhat.com; Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [PATCH v2] net/iavf: fix compilation warnings
> 
> 12/06/2023 10:32, Gao, DaxueX:
> > From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> > > Use fallthrough comments to ignore compilation warnings instead of
> > > diagnostic pragmas, which are not recognized by old GCC(below GCC7).
> > >
> > > Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on
> > > SSE")
> > > Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on
> > > AVX2")
> > > Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on
> > > AVX512")
> > > Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> > Tested-by: Daxue Gao <daxuex.gao@intel.com>
> 
> Please squash the fix in the original commits above, so we won't have any
> compilation issue when doing "git bisect".
> 
> In the meantime, I am pulling the branch next-net-intel without the Rx
> timestamp support for SSE/AVX2/AVX512.
> 

Squashed to original patches in dpdk-next-net-intel.


>
  
Thomas Monjalon June 12, 2023, 2:15 p.m. UTC | #4
12/06/2023 14:29, Zhang, Qi Z:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 12/06/2023 10:32, Gao, DaxueX:
> > > From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> > > > Use fallthrough comments to ignore compilation warnings instead of
> > > > diagnostic pragmas, which are not recognized by old GCC(below GCC7).
> > > >
> > > > Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on
> > > > SSE")
> > > > Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on
> > > > AVX2")
> > > > Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on
> > > > AVX512")
> > > > Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> > > Tested-by: Daxue Gao <daxuex.gao@intel.com>
> > 
> > Please squash the fix in the original commits above, so we won't have any
> > compilation issue when doing "git bisect".
> > 
> > In the meantime, I am pulling the branch next-net-intel without the Rx
> > timestamp support for SSE/AVX2/AVX512.
> > 
> 
> Squashed to original patches in dpdk-next-net-intel.

It is still not compiling:

In function '_mm_testz_si128',
    inlined from '_recv_raw_pkts_vec_flex_rxd' at ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1070:12:
/usr/lib/gcc/x86_64-pc-linux-gnu/13.1.1/include/smmintrin.h:69:10: error: 'mask_low' may be used uninitialized [-Werror=maybe-uninitialized]
   69 |   return __builtin_ia32_ptestz128 ((__v2di)__M, (__v2di)__V);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c: In function '_recv_raw_pkts_vec_flex_rxd':
../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1058:49: note: 'mask_low' was declared here
 1058 |                                         __m128i mask_low;
      |                                                 ^~~~~~~~
In function '_mm_testz_si128',
    inlined from '_recv_raw_pkts_vec_flex_rxd' at ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1072:13:
/usr/lib/gcc/x86_64-pc-linux-gnu/13.1.1/include/smmintrin.h:69:10: error: 'mask_high' may be used uninitialized [-Werror=maybe-uninitialized]
   69 |   return __builtin_ia32_ptestz128 ((__v2di)__M, (__v2di)__V);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c: In function '_recv_raw_pkts_vec_flex_rxd':
../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1059:49: note: 'mask_high' was declared here
 1059 |                                         __m128i mask_high;
      |                                                 ^~~~~~~~~
cc1: all warnings being treated as errors
  
Qi Zhang June 12, 2023, 3:08 p.m. UTC | #5
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, June 12, 2023 10:15 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Zeng, ZhichaoX <zhichaox.zeng@intel.com>; dev@dpdk.org; Richardson,
> Bruce <bruce.richardson@intel.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>;
> Gao, DaxueX <daxuex.gao@intel.com>; david.marchand@redhat.com;
> Mcnamara, John <john.mcnamara@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>
> Subject: Re: [PATCH v2] net/iavf: fix compilation warnings
> 
> 12/06/2023 14:29, Zhang, Qi Z:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 12/06/2023 10:32, Gao, DaxueX:
> > > > From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> > > > > Use fallthrough comments to ignore compilation warnings instead
> > > > > of diagnostic pragmas, which are not recognized by old GCC(below
> GCC7).
> > > > >
> > > > > Fixes: 95a1b0b23140 ("net/iavf: support Rx timestamp offload on
> > > > > SSE")
> > > > > Fixes: 4f58266da4d3 ("net/iavf: support Rx timestamp offload on
> > > > > AVX2")
> > > > > Fixes: d65eadb9e178 ("net/iavf: support Rx timestamp offload on
> > > > > AVX512")
> > > > > Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> > > > Tested-by: Daxue Gao <daxuex.gao@intel.com>
> > >
> > > Please squash the fix in the original commits above, so we won't
> > > have any compilation issue when doing "git bisect".
> > >
> > > In the meantime, I am pulling the branch next-net-intel without the
> > > Rx timestamp support for SSE/AVX2/AVX512.
> > >
> >
> > Squashed to original patches in dpdk-next-net-intel.
> 
> It is still not compiling:
> 
> In function '_mm_testz_si128',
>     inlined from '_recv_raw_pkts_vec_flex_rxd'
> at ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1070:12:
> /usr/lib/gcc/x86_64-pc-linux-gnu/13.1.1/include/smmintrin.h:69:10: error:
> 'mask_low' may be used uninitialized [-Werror=maybe-uninitialized]
>    69 |   return __builtin_ia32_ptestz128 ((__v2di)__M, (__v2di)__V);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c: In function
> '_recv_raw_pkts_vec_flex_rxd':
> ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1058:49: note: 'mask_low' was
> declared here
>  1058 |                                         __m128i mask_low;
>       |                                                 ^~~~~~~~
> In function '_mm_testz_si128',
>     inlined from '_recv_raw_pkts_vec_flex_rxd'
> at ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1072:13:
> /usr/lib/gcc/x86_64-pc-linux-gnu/13.1.1/include/smmintrin.h:69:10: error:
> 'mask_high' may be used uninitialized [-Werror=maybe-uninitialized]
>    69 |   return __builtin_ia32_ptestz128 ((__v2di)__M, (__v2di)__V);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c: In function
> '_recv_raw_pkts_vec_flex_rxd':
> ../../dpdk/drivers/net/iavf/iavf_rxtx_vec_sse.c:1059:49: note: 'mask_high' was
> declared here
>  1059 |                                         __m128i mask_high;
>       |                                                 ^~~~~~~~~
> cc1: all warnings being treated as errors
> 
> 

The question is if "apply patch failure" happens,  the CI build will not be executed and we may not able to figure out the error early, any BKM we should follow? Thanks.
  

Patch

diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx2.c b/drivers/net/iavf/iavf_rxtx_vec_avx2.c
index 86290c4bbb..184fa30e0e 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_avx2.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_avx2.c
@@ -1397,36 +1397,42 @@  _iavf_recv_raw_pkts_vec_avx2_flex_rxd(struct iavf_rx_queue *rxq,
 					(_mm256_castsi256_si128(status0_7)));
 		received += burst;
 #ifndef RTE_LIBRTE_IAVF_16BYTE_RX_DESC
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
 		if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
 			inflection_point = (inflection_point <= burst) ? inflection_point : 0;
 			switch (inflection_point) {
 			case 1:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 2:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 1],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 3:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 2],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 4:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 3],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 5:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 4],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 6:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 5],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 7:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 6],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 8:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 7],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
 				rxq->phc_time += (uint64_t)1 << 32;
+				/* fallthrough */
 			case 0:
 				break;
 			default:
@@ -1436,7 +1442,6 @@  _iavf_recv_raw_pkts_vec_avx2_flex_rxd(struct iavf_rx_queue *rxq,
 
 			rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
 		}
-#pragma GCC diagnostic pop
 #endif
 		if (burst != IAVF_DESCS_PER_LOOP_AVX)
 			break;
diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx512.c b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
index f9961e53b8..38649b6f0b 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_avx512.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
@@ -1551,36 +1551,42 @@  _iavf_recv_raw_pkts_vec_avx512_flex_rxd(struct iavf_rx_queue *rxq,
 		received += burst;
 #ifndef RTE_LIBRTE_IAVF_16BYTE_RX_DESC
 #ifdef IAVF_RX_TS_OFFLOAD
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
 		if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
 			inflection_point = (inflection_point <= burst) ? inflection_point : 0;
 			switch (inflection_point) {
 			case 1:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 2:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 1],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 3:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 2],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 4:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 3],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 5:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 4],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 6:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 5],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 7:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 6],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 8:
 				*RTE_MBUF_DYNFIELD(rx_pkts[i + 7],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
 				rxq->phc_time += (uint64_t)1 << 32;
+				/* fallthrough */
 			case 0:
 				break;
 			default:
@@ -1590,7 +1596,6 @@  _iavf_recv_raw_pkts_vec_avx512_flex_rxd(struct iavf_rx_queue *rxq,
 
 			rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
 		}
-#pragma GCC diagnostic pop
 #endif
 #endif
 		if (burst != IAVF_DESCS_PER_LOOP_AVX)
diff --git a/drivers/net/iavf/iavf_rxtx_vec_sse.c b/drivers/net/iavf/iavf_rxtx_vec_sse.c
index b754122c51..81ea154519 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_sse.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_sse.c
@@ -1126,24 +1126,26 @@  _recv_raw_pkts_vec_flex_rxd(struct iavf_rx_queue *rxq,
 		nb_pkts_recd += var;
 
 #ifndef RTE_LIBRTE_IAVF_16BYTE_RX_DESC
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wimplicit-fallthrough"
 		if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
 			inflection_point = (inflection_point <= var) ? inflection_point : 0;
 			switch (inflection_point) {
 			case 1:
 				*RTE_MBUF_DYNFIELD(rx_pkts[pos + 0],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 2:
 				*RTE_MBUF_DYNFIELD(rx_pkts[pos + 1],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 3:
 				*RTE_MBUF_DYNFIELD(rx_pkts[pos + 2],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
+				/* fallthrough */
 			case 4:
 				*RTE_MBUF_DYNFIELD(rx_pkts[pos + 3],
 					iavf_timestamp_dynfield_offset + 4, uint32_t *) += 1;
 				rxq->phc_time += (uint64_t)1 << 32;
+				/* fallthrough */
 			case 0:
 				break;
 			default:
@@ -1153,7 +1155,6 @@  _recv_raw_pkts_vec_flex_rxd(struct iavf_rx_queue *rxq,
 
 			rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
 		}
-#pragma GCC diagnostic pop
 #endif
 
 		if (likely(var != IAVF_VPMD_DESCS_PER_LOOP))