[v2,3/4] net/mlx5: remove weak stub functions

Message ID 20230112203719.1528619-4-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/4] eal/windows: fix pedantic build |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon Jan. 12, 2023, 8:37 p.m. UTC
  The vector Rx functions are conditionally compiled.
Some stub functions were also always compiled with weak attribute.
If there is no vector support, the weak functions were linked.

These weak functions are moved in a specific file
which is compiled only if there is no vector support.
This way it is simpler to understand,
and the weak attributes can be removed.

This change helps to compile with MinGW GCC
which has no support for weak functions.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/mlx5/meson.build          | 13 +++++----
 drivers/net/mlx5/mlx5_rx.c            | 35 ------------------------
 drivers/net/mlx5/mlx5_rxtx_vec_null.c | 38 +++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 40 deletions(-)
 create mode 100644 drivers/net/mlx5/mlx5_rxtx_vec_null.c
  

Comments

Dmitry Kozlyuk Jan. 23, 2023, 7:42 p.m. UTC | #1
2023-01-12 21:37 (UTC+0100), Thomas Monjalon:
> The vector Rx functions are conditionally compiled.
> Some stub functions were also always compiled with weak attribute.
> If there is no vector support, the weak functions were linked.
> 
> These weak functions are moved in a specific file
> which is compiled only if there is no vector support.
> This way it is simpler to understand,
> and the weak attributes can be removed.
> 
> This change helps to compile with MinGW GCC
> which has no support for weak functions.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  drivers/net/mlx5/meson.build          | 13 +++++----
>  drivers/net/mlx5/mlx5_rx.c            | 35 ------------------------
>  drivers/net/mlx5/mlx5_rxtx_vec_null.c | 38 +++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 40 deletions(-)
>  create mode 100644 drivers/net/mlx5/mlx5_rxtx_vec_null.c
> 
> diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> index abd507bd88..dba911693e 100644
> --- a/drivers/net/mlx5/meson.build
> +++ b/drivers/net/mlx5/meson.build
> @@ -49,11 +49,14 @@ if is_linux
>              'mlx5_hws_cnt.c',
>              'mlx5_flow_verbs.c',
>      )
> -    if (dpdk_conf.has('RTE_ARCH_X86_64')
> -        or dpdk_conf.has('RTE_ARCH_ARM64')
> -        or dpdk_conf.has('RTE_ARCH_PPC_64'))
> -        sources += files('mlx5_rxtx_vec.c')
> -    endif
> +endif
> +
> +if is_linux and (dpdk_conf.has('RTE_ARCH_X86_64')
> +              or dpdk_conf.has('RTE_ARCH_ARM64')
> +              or dpdk_conf.has('RTE_ARCH_PPC_64'))
> +    sources += files('mlx5_rxtx_vec.c')
> +else
> +    sources += files('mlx5_rxtx_vec_null.c')
>  endif

Can "is_linux" be dropped now?
I suspect that vector routines were compiled only for Linux
to prevent linker errors from both weak and non-weak symbols present,
not because vector code is somehow Linux-specific.
  
Thomas Monjalon Jan. 24, 2023, 2:42 p.m. UTC | #2
23/01/2023 20:42, Dmitry Kozlyuk:
> 2023-01-12 21:37 (UTC+0100), Thomas Monjalon:
> > The vector Rx functions are conditionally compiled.
> > Some stub functions were also always compiled with weak attribute.
> > If there is no vector support, the weak functions were linked.
> > 
> > These weak functions are moved in a specific file
> > which is compiled only if there is no vector support.
> > This way it is simpler to understand,
> > and the weak attributes can be removed.
> > 
> > This change helps to compile with MinGW GCC
> > which has no support for weak functions.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  drivers/net/mlx5/meson.build          | 13 +++++----
> >  drivers/net/mlx5/mlx5_rx.c            | 35 ------------------------
> >  drivers/net/mlx5/mlx5_rxtx_vec_null.c | 38 +++++++++++++++++++++++++++
> >  3 files changed, 46 insertions(+), 40 deletions(-)
> >  create mode 100644 drivers/net/mlx5/mlx5_rxtx_vec_null.c
> > 
> > diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> > index abd507bd88..dba911693e 100644
> > --- a/drivers/net/mlx5/meson.build
> > +++ b/drivers/net/mlx5/meson.build
> > @@ -49,11 +49,14 @@ if is_linux
> >              'mlx5_hws_cnt.c',
> >              'mlx5_flow_verbs.c',
> >      )
> > -    if (dpdk_conf.has('RTE_ARCH_X86_64')
> > -        or dpdk_conf.has('RTE_ARCH_ARM64')
> > -        or dpdk_conf.has('RTE_ARCH_PPC_64'))
> > -        sources += files('mlx5_rxtx_vec.c')
> > -    endif
> > +endif
> > +
> > +if is_linux and (dpdk_conf.has('RTE_ARCH_X86_64')
> > +              or dpdk_conf.has('RTE_ARCH_ARM64')
> > +              or dpdk_conf.has('RTE_ARCH_PPC_64'))
> > +    sources += files('mlx5_rxtx_vec.c')
> > +else
> > +    sources += files('mlx5_rxtx_vec_null.c')
> >  endif
> 
> Can "is_linux" be dropped now?
> I suspect that vector routines were compiled only for Linux
> to prevent linker errors from both weak and non-weak symbols present,
> not because vector code is somehow Linux-specific.

Maybe but we should enable vector routines on Windows in a separate patch.
  

Patch

diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index abd507bd88..dba911693e 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -49,11 +49,14 @@  if is_linux
             'mlx5_hws_cnt.c',
             'mlx5_flow_verbs.c',
     )
-    if (dpdk_conf.has('RTE_ARCH_X86_64')
-        or dpdk_conf.has('RTE_ARCH_ARM64')
-        or dpdk_conf.has('RTE_ARCH_PPC_64'))
-        sources += files('mlx5_rxtx_vec.c')
-    endif
+endif
+
+if is_linux and (dpdk_conf.has('RTE_ARCH_X86_64')
+              or dpdk_conf.has('RTE_ARCH_ARM64')
+              or dpdk_conf.has('RTE_ARCH_PPC_64'))
+    sources += files('mlx5_rxtx_vec.c')
+else
+    sources += files('mlx5_rxtx_vec_null.c')
 endif
 
 cflags_options = [
diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
index 917c517b83..d054f9728a 100644
--- a/drivers/net/mlx5/mlx5_rx.c
+++ b/drivers/net/mlx5/mlx5_rx.c
@@ -1185,41 +1185,6 @@  mlx5_rx_burst_mprq(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 	return i;
 }
 
-/*
- * Vectorized Rx routines are not compiled in when required vector instructions
- * are not supported on a target architecture.
- * The following null stubs are needed for linkage when those are not included
- * outside of this file (e.g. mlx5_rxtx_vec_sse.c for x86).
- */
-
-__rte_weak uint16_t
-mlx5_rx_burst_vec(void *dpdk_rxq __rte_unused,
-		  struct rte_mbuf **pkts __rte_unused,
-		  uint16_t pkts_n __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak uint16_t
-mlx5_rx_burst_mprq_vec(void *dpdk_rxq __rte_unused,
-		       struct rte_mbuf **pkts __rte_unused,
-		       uint16_t pkts_n __rte_unused)
-{
-	return 0;
-}
-
-__rte_weak int
-mlx5_rxq_check_vec_support(struct mlx5_rxq_data *rxq __rte_unused)
-{
-	return -ENOTSUP;
-}
-
-__rte_weak int
-mlx5_check_vec_rx_support(struct rte_eth_dev *dev __rte_unused)
-{
-	return -ENOTSUP;
-}
-
 int
 mlx5_rx_queue_lwm_query(struct rte_eth_dev *dev,
 			uint16_t *queue_id, uint8_t *lwm)
diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_null.c b/drivers/net/mlx5/mlx5_rxtx_vec_null.c
new file mode 100644
index 0000000000..03d6629e24
--- /dev/null
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_null.c
@@ -0,0 +1,38 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2023 NVIDIA Corporation & Affiliates
+ */
+
+#include <rte_common.h>
+
+#include "mlx5_rx.h"
+
+struct rte_mbuf;
+struct rte_eth_dev;
+
+uint16_t
+mlx5_rx_burst_vec(void *dpdk_rxq __rte_unused,
+		struct rte_mbuf **pkts __rte_unused,
+		uint16_t pkts_n __rte_unused)
+{
+	return 0;
+}
+
+uint16_t
+mlx5_rx_burst_mprq_vec(void *dpdk_rxq __rte_unused,
+		struct rte_mbuf **pkts __rte_unused,
+		uint16_t pkts_n __rte_unused)
+{
+	return 0;
+}
+
+int
+mlx5_rxq_check_vec_support(struct mlx5_rxq_data *rxq __rte_unused)
+{
+	return -ENOTSUP;
+}
+
+int
+mlx5_check_vec_rx_support(struct rte_eth_dev *dev __rte_unused)
+{
+	return -ENOTSUP;
+}