[v2,1/2] build: build only one library type on Windows
Checks
Commit Message
MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.
Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.
For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
app/meson.build | 6 +++
config/meson.build | 27 ++++++++++++
drivers/meson.build | 72 +++++++++++++++----------------
drivers/net/octeontx/base/meson.build | 2 +-
examples/meson.build | 6 +++
lib/meson.build | 79 +++++++++++++++--------------------
6 files changed, 108 insertions(+), 84 deletions(-)
Comments
On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> MSVC is the only compiler that can produce usable shared libraries for
> DPDK on Windows because of the use of exported TLS variables.
>
> Disable building of shared libraries with LLVM and MinGW so that
> remaining __declspec macros needed for the functional libraries built by
> MSVC can be used without triggering errors in LLVM and MinGW builds.
>
> For Windows only install the default_library type to avoid confusion.
> Windows builds cannot build both shared and static in a single pass so
> install only the functional variant.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Comments inline below.
/Bruce
> ---
> app/meson.build | 6 +++
> config/meson.build | 27 ++++++++++++
> drivers/meson.build | 72 +++++++++++++++----------------
> drivers/net/octeontx/base/meson.build | 2 +-
> examples/meson.build | 6 +++
> lib/meson.build | 79 +++++++++++++++--------------------
> 6 files changed, 108 insertions(+), 84 deletions(-)
>
> diff --git a/app/meson.build b/app/meson.build
> index 21b6da2..f4ed0f1 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> default_ldflags = []
> if get_option('default_library') == 'static' and not is_windows
> default_ldflags += ['-Wl,--export-dynamic']
> +elif get_option('default_library') == 'shared' and is_ms_compiler
> + default_ldflags += ['/experimental:tlsDllInterface']
> endif
>
Is this necessary, given that you add this flag as a project-wide argument
below in config/meson.build?
> foreach app:apps
> @@ -104,6 +106,10 @@ foreach app:apps
> link_libs = dpdk_static_libraries + dpdk_drivers
> endif
>
> + if is_windows and is_shared_enabled
> + cflags += '-DRTE_BUILD_SHARED_LIB'
> + endif
> +
> exec = executable('dpdk-' + name,
> sources,
> c_args: cflags,
> diff --git a/config/meson.build b/config/meson.build
> index 8c8b019..dd7971e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -53,6 +53,9 @@ if is_ms_compiler
>
> # enable statement expressions extension
> add_project_arguments('/experimental:statementExpressions', language: 'c')
> +
> + # enable export of thread_local variables
> + add_project_arguments('/experimental:tlsDllInterface', language: 'c')
> endif
>
> # set the major version, which might be used by drivers and libraries
> @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
> NOTE: DPDK always builds both shared and static libraries. Please set
> "default_library" to either "static" or "shared" to select default linkage
> for apps and any examples.''')
> +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> + error( '''
> + Unsupported value "shared" for "default_library" option.
> +
> + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> + "default_library" to either "static" or use MSVC.''')
> +endif
> +
> +if is_windows
> + if is_ms_compiler and get_option('default_library') == 'shared'
> + is_shared_enabled=true
> + else
> + is_shared_enabled=false
> + endif
> +else
> + is_shared_enabled=true
> +endif
> +
This can be shortened to:
is_shared_enabled=true
if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
is_shared_enabled=false
endif
> +if is_windows
> + install_static = get_option('default_library') == 'static'
> + install_shared = get_option('default_library') == 'shared'
> +else
> + install_static=true
> + install_shared=true
> endif
Might be better merged with the block above:
is_shared_enabled = true
install_static = true
install_shared = true
if is_windows
install_static = get_option('default_library') == 'static'
install_shared = get_option('default_library') == 'shared'
if not is_ms_compiler or not install_shared
is_shared_enabled = false
endif
endif
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 66931d4..c8b2d13 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -80,7 +80,7 @@ foreach subpath:subdirs
> subdir(class)
> skip_class = false
> foreach d:std_deps
> - if not is_variable('shared_rte_' + d)
> + if not is_variable('static_rte_' + d)
> skip_class = true
> reason = 'missing internal dependency, "@0@"'.format(d)
> if dpdk_libs_deprecated.contains(d)
> @@ -173,7 +173,7 @@ foreach subpath:subdirs
> if not build
> break
> endif
> - if not is_variable('shared_rte_' + d)
> + if not is_variable('static_rte_' + d)
> build = false
> reason = 'missing internal dependency, "@0@"'.format(d)
> if dpdk_libs_deprecated.contains(d)
> @@ -188,7 +188,9 @@ foreach subpath:subdirs
> +'\tPlease enable missing dependency "@0@"'.format(d))
> endif
> else
> - shared_deps += [get_variable('shared_rte_' + d)]
> + if is_shared_enabled
> + shared_deps += [get_variable('shared_rte_' + d)]
> + endif
> static_deps += [get_variable('static_rte_' + d)]
> endif
> endforeach
> @@ -208,6 +210,9 @@ foreach subpath:subdirs
> enabled_drivers += name
> lib_name = '_'.join(['rte', class, name])
> cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> + if is_windows and is_shared_enabled
> + cflags += '-DRTE_BUILD_SHARED_LIB'
> + endif
Rather than setting this in cflags in the drivers or lib meson.build files,
can we set this once as a project-level cflag and be done with it?
> if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
> cflags += '-DRTE_ANNOTATE_LOCKS'
> cflags += '-Wthread-safety'
> @@ -247,7 +252,7 @@ foreach subpath:subdirs
> include_directories: includes,
> dependencies: static_deps,
> c_args: cflags,
> - install: true)
> + install: install_static)
>
> # now build the shared driver
> version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> @@ -271,55 +276,46 @@ foreach subpath:subdirs
> endif
> endif
>
> - if is_windows
> - if is_ms_linker
> + if is_shared_enabled
> + if is_ms_compiler
> def_file = custom_target(lib_name + '_def',
> command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> input: version_map,
> output: '@0@_exports.def'.format(lib_name))
> lk_deps += [def_file]
> -
> - lk_args = ['-Wl,/def:' + def_file.full_path()]
> - if meson.version().version_compare('<0.54.0')
> - lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> - endif
> + lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
Again, the /experimental flag shouldn't be necessary, since it's set as
project option in config/meson.build.
> else
> - mingw_map = custom_target(lib_name + '_mingw',
> - command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> - input: version_map,
> - output: '@0@_mingw.map'.format(lib_name))
> - lk_deps += [mingw_map]
> -
> - lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> + lk_args = ['-Wl,--version-script=' + version_map]
> endif
> - else
> - lk_args = ['-Wl,--version-script=' + version_map]
> endif
>
> - shared_lib = shared_library(lib_name, sources,
> - objects: objs,
> - include_directories: includes,
> - dependencies: shared_deps,
> - c_args: cflags,
> - link_args: lk_args,
> - link_depends: lk_deps,
> - version: abi_version,
> - soversion: so_version,
> - install: true,
> - install_dir: driver_install_path)
> -
> - # create a dependency object and add it to the global dictionary so
> - # testpmd or other built-in apps can find it if necessary
> - shared_dep = declare_dependency(link_with: shared_lib,
> - include_directories: includes,
> - dependencies: shared_deps)
> + if is_shared_enabled
> + shared_lib = shared_library(lib_name, sources,
> + objects: objs,
> + include_directories: includes,
> + dependencies: shared_deps,
> + c_args: cflags,
> + link_args: lk_args,
> + link_depends: lk_deps,
> + version: abi_version,
> + soversion: so_version,
> + install: install_shared,
> + install_dir: driver_install_path)
> +
> + # create a dependency object and add it to the global dictionary so
> + # testpmd or other built-in apps can find it if necessary
> + shared_dep = declare_dependency(link_with: shared_lib,
> + include_directories: includes,
> + dependencies: shared_deps)
> +
> + set_variable('shared_@0@'.format(lib_name), shared_dep)
Just a thought - could you avoid some of the changes throughout this set,
if you add an else-leg to the "is_shared_enabled" check, where, if shared
is not enabled, you assign shared_dep to be an empty string, or an empty
object or something.
That would mean that you wouldn't need to move the set_variable line as
here, you also wouldn't need to put conditionals around all the
Just a thought - could you avoid some of the changes throughout this set,
if you add an else-leg to the "is_shared_enabled" check, where, if shared
is not enabled, you assign shared_dep to be an empty string, or an empty
object or something.
That would mean that you wouldn't need to move the set_variable line as
here, you also wouldn't need to put conditionals around all the
'shared_deps +=' lines when processing deps for a component, and lastly,
you may not even need to change all the searches for shared_rte_ to
static_rte, since the variables would exist for shared - they'd just be
empty!
> + endif
> static_dep = declare_dependency(
> include_directories: includes,
> dependencies: static_deps)
>
> dpdk_drivers += static_lib
<snip for brevity>
On Fri, Apr 12, 2024 at 02:00:43PM +0100, Bruce Richardson wrote:
> On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> > MSVC is the only compiler that can produce usable shared libraries for
> > DPDK on Windows because of the use of exported TLS variables.
> >
> > Disable building of shared libraries with LLVM and MinGW so that
> > remaining __declspec macros needed for the functional libraries built by
> > MSVC can be used without triggering errors in LLVM and MinGW builds.
> >
> > For Windows only install the default_library type to avoid confusion.
> > Windows builds cannot build both shared and static in a single pass so
> > install only the functional variant.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>
> Comments inline below.
>
> /Bruce
>
> > ---
> > app/meson.build | 6 +++
> > config/meson.build | 27 ++++++++++++
> > drivers/meson.build | 72 +++++++++++++++----------------
> > drivers/net/octeontx/base/meson.build | 2 +-
> > examples/meson.build | 6 +++
> > lib/meson.build | 79 +++++++++++++++--------------------
> > 6 files changed, 108 insertions(+), 84 deletions(-)
> >
> > diff --git a/app/meson.build b/app/meson.build
> > index 21b6da2..f4ed0f1 100644
> > --- a/app/meson.build
> > +++ b/app/meson.build
> > @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> > default_ldflags = []
> > if get_option('default_library') == 'static' and not is_windows
> > default_ldflags += ['-Wl,--export-dynamic']
> > +elif get_option('default_library') == 'shared' and is_ms_compiler
> > + default_ldflags += ['/experimental:tlsDllInterface']
> > endif
> >
>
> Is this necessary, given that you add this flag as a project-wide argument
> below in config/meson.build?
yes, i need to provide tlsDllInterface as an argument to both the
compiler and the linker commands. project-wide argument from
config/meson.build only adds the argument to the compiler command line
but not the linker. there doesn't appear to be a global variable that
can be used as the same for the linker so it has been added to the
various ldflags/lkflags variables in individual meson.build files.
> > foreach app:apps
> > @@ -104,6 +106,10 @@ foreach app:apps
> > link_libs = dpdk_static_libraries + dpdk_drivers
> > endif
> >
> > + if is_windows and is_shared_enabled
> > + cflags += '-DRTE_BUILD_SHARED_LIB'
> > + endif
> > +
> > exec = executable('dpdk-' + name,
> > sources,
> > c_args: cflags,
> > diff --git a/config/meson.build b/config/meson.build
> > index 8c8b019..dd7971e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -53,6 +53,9 @@ if is_ms_compiler
> >
> > # enable statement expressions extension
> > add_project_arguments('/experimental:statementExpressions', language: 'c')
> > +
> > + # enable export of thread_local variables
> > + add_project_arguments('/experimental:tlsDllInterface', language: 'c')
> > endif
as per above, this only causes the argument to be added to the compiler
not the linker.
> >
> > # set the major version, which might be used by drivers and libraries
> > @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
> > NOTE: DPDK always builds both shared and static libraries. Please set
> > "default_library" to either "static" or "shared" to select default linkage
> > for apps and any examples.''')
> > +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> > + error( '''
> > + Unsupported value "shared" for "default_library" option.
> > +
> > + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> > + "default_library" to either "static" or use MSVC.''')
> > +endif
> > +
> > +if is_windows
> > + if is_ms_compiler and get_option('default_library') == 'shared'
> > + is_shared_enabled=true
> > + else
> > + is_shared_enabled=false
> > + endif
> > +else
> > + is_shared_enabled=true
> > +endif
> > +
>
> This can be shortened to:
>
> is_shared_enabled=true
> if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
> is_shared_enabled=false
> endif
>
>
> > +if is_windows
> > + install_static = get_option('default_library') == 'static'
> > + install_shared = get_option('default_library') == 'shared'
> > +else
> > + install_static=true
> > + install_shared=true
> > endif
>
> Might be better merged with the block above:
>
> is_shared_enabled = true
> install_static = true
> install_shared = true
> if is_windows
> install_static = get_option('default_library') == 'static'
> install_shared = get_option('default_library') == 'shared'
>
> if not is_ms_compiler or not install_shared
> is_shared_enabled = false
> endif
> endif
done in v3, much nicer thanks.
>
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index 66931d4..c8b2d13 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -80,7 +80,7 @@ foreach subpath:subdirs
> > subdir(class)
> > skip_class = false
> > foreach d:std_deps
> > - if not is_variable('shared_rte_' + d)
> > + if not is_variable('static_rte_' + d)
> > skip_class = true
> > reason = 'missing internal dependency, "@0@"'.format(d)
> > if dpdk_libs_deprecated.contains(d)
> > @@ -173,7 +173,7 @@ foreach subpath:subdirs
> > if not build
> > break
> > endif
> > - if not is_variable('shared_rte_' + d)
> > + if not is_variable('static_rte_' + d)
> > build = false
> > reason = 'missing internal dependency, "@0@"'.format(d)
> > if dpdk_libs_deprecated.contains(d)
> > @@ -188,7 +188,9 @@ foreach subpath:subdirs
> > +'\tPlease enable missing dependency "@0@"'.format(d))
> > endif
> > else
> > - shared_deps += [get_variable('shared_rte_' + d)]
> > + if is_shared_enabled
> > + shared_deps += [get_variable('shared_rte_' + d)]
> > + endif
> > static_deps += [get_variable('static_rte_' + d)]
> > endif
> > endforeach
> > @@ -208,6 +210,9 @@ foreach subpath:subdirs
> > enabled_drivers += name
> > lib_name = '_'.join(['rte', class, name])
> > cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> > + if is_windows and is_shared_enabled
> > + cflags += '-DRTE_BUILD_SHARED_LIB'
> > + endif
>
> Rather than setting this in cflags in the drivers or lib meson.build files,
> can we set this once as a project-level cflag and be done with it?
i don't think so, there are drivers (notably mellanox) that appear to
assume it is not always set by default even with a shared build. i think
this one might best be cleaned up and clarified in a separate series.
>
> > if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
> > cflags += '-DRTE_ANNOTATE_LOCKS'
> > cflags += '-Wthread-safety'
> > @@ -247,7 +252,7 @@ foreach subpath:subdirs
> > include_directories: includes,
> > dependencies: static_deps,
> > c_args: cflags,
> > - install: true)
> > + install: install_static)
> >
> > # now build the shared driver
> > version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> > @@ -271,55 +276,46 @@ foreach subpath:subdirs
> > endif
> > endif
> >
> > - if is_windows
> > - if is_ms_linker
> > + if is_shared_enabled
> > + if is_ms_compiler
> > def_file = custom_target(lib_name + '_def',
> > command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > input: version_map,
> > output: '@0@_exports.def'.format(lib_name))
> > lk_deps += [def_file]
> > -
> > - lk_args = ['-Wl,/def:' + def_file.full_path()]
> > - if meson.version().version_compare('<0.54.0')
> > - lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> > - endif
> > + lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
>
> Again, the /experimental flag shouldn't be necessary, since it's set as
> project option in config/meson.build.
as per above, let me know if what i said seems crazy.
>
> > else
> > - mingw_map = custom_target(lib_name + '_mingw',
> > - command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > - input: version_map,
> > - output: '@0@_mingw.map'.format(lib_name))
> > - lk_deps += [mingw_map]
> > -
> > - lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> > + lk_args = ['-Wl,--version-script=' + version_map]
> > endif
> > - else
> > - lk_args = ['-Wl,--version-script=' + version_map]
> > endif
> >
> > - shared_lib = shared_library(lib_name, sources,
> > - objects: objs,
> > - include_directories: includes,
> > - dependencies: shared_deps,
> > - c_args: cflags,
> > - link_args: lk_args,
> > - link_depends: lk_deps,
> > - version: abi_version,
> > - soversion: so_version,
> > - install: true,
> > - install_dir: driver_install_path)
> > -
> > - # create a dependency object and add it to the global dictionary so
> > - # testpmd or other built-in apps can find it if necessary
> > - shared_dep = declare_dependency(link_with: shared_lib,
> > - include_directories: includes,
> > - dependencies: shared_deps)
> > + if is_shared_enabled
> > + shared_lib = shared_library(lib_name, sources,
> > + objects: objs,
> > + include_directories: includes,
> > + dependencies: shared_deps,
> > + c_args: cflags,
> > + link_args: lk_args,
> > + link_depends: lk_deps,
> > + version: abi_version,
> > + soversion: so_version,
> > + install: install_shared,
> > + install_dir: driver_install_path)
> > +
> > + # create a dependency object and add it to the global dictionary so
> > + # testpmd or other built-in apps can find it if necessary
> > + shared_dep = declare_dependency(link_with: shared_lib,
> > + include_directories: includes,
> > + dependencies: shared_deps)
> > +
> > + set_variable('shared_@0@'.format(lib_name), shared_dep)
>
> Just a thought - could you avoid some of the changes throughout this set,
> if you add an else-leg to the "is_shared_enabled" check, where, if shared
> is not enabled, you assign shared_dep to be an empty string, or an empty
> object or something.
>
> That would mean that you wouldn't need to move the set_variable line as
> here, you also wouldn't need to put conditionals around all the
>
> Just a thought - could you avoid some of the changes throughout this set,
> if you add an else-leg to the "is_shared_enabled" check, where, if shared
> is not enabled, you assign shared_dep to be an empty string, or an empty
> object or something.
>
> That would mean that you wouldn't need to move the set_variable line as
> here, you also wouldn't need to put conditionals around all the
> 'shared_deps +=' lines when processing deps for a component, and lastly,
> you may not even need to change all the searches for shared_rte_ to
> static_rte, since the variables would exist for shared - they'd just be
> empty!
did this in v3, still requires a diff -b to read what actually changed
but appears to have worked out as you anticipated. so i think probably
it was a good improvement.
>
> > + endif
> > static_dep = declare_dependency(
> > include_directories: includes,
> > dependencies: static_deps)
> >
> > dpdk_drivers += static_lib
>
> <snip for brevity>
On Fri, Apr 12, 2024 at 04:04:12PM -0700, Tyler Retzlaff wrote:
> On Fri, Apr 12, 2024 at 02:00:43PM +0100, Bruce Richardson wrote:
> > On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> > > MSVC is the only compiler that can produce usable shared libraries for
> > > DPDK on Windows because of the use of exported TLS variables.
> > >
> > > Disable building of shared libraries with LLVM and MinGW so that
> > > remaining __declspec macros needed for the functional libraries built by
> > > MSVC can be used without triggering errors in LLVM and MinGW builds.
> > >
> > > For Windows only install the default_library type to avoid confusion.
> > > Windows builds cannot build both shared and static in a single pass so
> > > install only the functional variant.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >
> > Comments inline below.
> >
> > /Bruce
> >
> > > ---
> > > app/meson.build | 6 +++
> > > config/meson.build | 27 ++++++++++++
> > > drivers/meson.build | 72 +++++++++++++++----------------
> > > drivers/net/octeontx/base/meson.build | 2 +-
> > > examples/meson.build | 6 +++
> > > lib/meson.build | 79 +++++++++++++++--------------------
> > > 6 files changed, 108 insertions(+), 84 deletions(-)
> > >
> > > diff --git a/app/meson.build b/app/meson.build
> > > index 21b6da2..f4ed0f1 100644
> > > --- a/app/meson.build
> > > +++ b/app/meson.build
> > > @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> > > default_ldflags = []
> > > if get_option('default_library') == 'static' and not is_windows
> > > default_ldflags += ['-Wl,--export-dynamic']
> > > +elif get_option('default_library') == 'shared' and is_ms_compiler
> > > + default_ldflags += ['/experimental:tlsDllInterface']
> > > endif
> > >
> >
> > Is this necessary, given that you add this flag as a project-wide argument
> > below in config/meson.build?
>
> yes, i need to provide tlsDllInterface as an argument to both the
> compiler and the linker commands. project-wide argument from
> config/meson.build only adds the argument to the compiler command line
> but not the linker. there doesn't appear to be a global variable that
> can be used as the same for the linker so it has been added to the
> various ldflags/lkflags variables in individual meson.build files.
>
The add_project_link_arguments() function should be what you are looking
for, I think.
https://mesonbuild.com/Reference-manual_functions.html#add_project_link_arguments
> > > foreach app:apps
> > > @@ -104,6 +106,10 @@ foreach app:apps
> > > link_libs = dpdk_static_libraries + dpdk_drivers
> > > endif
> > >
> > > + if is_windows and is_shared_enabled
> > > + cflags += '-DRTE_BUILD_SHARED_LIB'
> > > + endif
> > > +
> > > exec = executable('dpdk-' + name,
> > > sources,
> > > c_args: cflags,
> > > diff --git a/config/meson.build b/config/meson.build
> > > index 8c8b019..dd7971e 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -53,6 +53,9 @@ if is_ms_compiler
> > >
> > > # enable statement expressions extension
> > > add_project_arguments('/experimental:statementExpressions', language: 'c')
> > > +
> > > + # enable export of thread_local variables
> > > + add_project_arguments('/experimental:tlsDllInterface', language: 'c')
> > > endif
>
> as per above, this only causes the argument to be added to the compiler
> not the linker.
>
> > >
> > > # set the major version, which might be used by drivers and libraries
> > > @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
> > > NOTE: DPDK always builds both shared and static libraries. Please set
> > > "default_library" to either "static" or "shared" to select default linkage
> > > for apps and any examples.''')
> > > +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> > > + error( '''
> > > + Unsupported value "shared" for "default_library" option.
> > > +
> > > + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> > > + "default_library" to either "static" or use MSVC.''')
> > > +endif
> > > +
> > > +if is_windows
> > > + if is_ms_compiler and get_option('default_library') == 'shared'
> > > + is_shared_enabled=true
> > > + else
> > > + is_shared_enabled=false
> > > + endif
> > > +else
> > > + is_shared_enabled=true
> > > +endif
> > > +
> >
> > This can be shortened to:
> >
> > is_shared_enabled=true
> > if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
> > is_shared_enabled=false
> > endif
> >
> >
> > > +if is_windows
> > > + install_static = get_option('default_library') == 'static'
> > > + install_shared = get_option('default_library') == 'shared'
> > > +else
> > > + install_static=true
> > > + install_shared=true
> > > endif
> >
> > Might be better merged with the block above:
> >
> > is_shared_enabled = true
> > install_static = true
> > install_shared = true
> > if is_windows
> > install_static = get_option('default_library') == 'static'
> > install_shared = get_option('default_library') == 'shared'
> >
> > if not is_ms_compiler or not install_shared
> > is_shared_enabled = false
> > endif
> > endif
>
> done in v3, much nicer thanks.
>
> >
> > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > index 66931d4..c8b2d13 100644
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > @@ -80,7 +80,7 @@ foreach subpath:subdirs
> > > subdir(class)
> > > skip_class = false
> > > foreach d:std_deps
> > > - if not is_variable('shared_rte_' + d)
> > > + if not is_variable('static_rte_' + d)
> > > skip_class = true
> > > reason = 'missing internal dependency, "@0@"'.format(d)
> > > if dpdk_libs_deprecated.contains(d)
> > > @@ -173,7 +173,7 @@ foreach subpath:subdirs
> > > if not build
> > > break
> > > endif
> > > - if not is_variable('shared_rte_' + d)
> > > + if not is_variable('static_rte_' + d)
> > > build = false
> > > reason = 'missing internal dependency, "@0@"'.format(d)
> > > if dpdk_libs_deprecated.contains(d)
> > > @@ -188,7 +188,9 @@ foreach subpath:subdirs
> > > +'\tPlease enable missing dependency "@0@"'.format(d))
> > > endif
> > > else
> > > - shared_deps += [get_variable('shared_rte_' + d)]
> > > + if is_shared_enabled
> > > + shared_deps += [get_variable('shared_rte_' + d)]
> > > + endif
> > > static_deps += [get_variable('static_rte_' + d)]
> > > endif
> > > endforeach
> > > @@ -208,6 +210,9 @@ foreach subpath:subdirs
> > > enabled_drivers += name
> > > lib_name = '_'.join(['rte', class, name])
> > > cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> > > + if is_windows and is_shared_enabled
> > > + cflags += '-DRTE_BUILD_SHARED_LIB'
> > > + endif
> >
> > Rather than setting this in cflags in the drivers or lib meson.build files,
> > can we set this once as a project-level cflag and be done with it?
>
> i don't think so, there are drivers (notably mellanox) that appear to
> assume it is not always set by default even with a shared build. i think
> this one might best be cleaned up and clarified in a separate series.
>
Oh, how does this assumption show itself or what impact has it? Using git
grep in the source code for the macro only shows the expected usage in
function_versioning header, and in lib/meson.build.
> >
> > > if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
> > > cflags += '-DRTE_ANNOTATE_LOCKS'
> > > cflags += '-Wthread-safety'
> > > @@ -247,7 +252,7 @@ foreach subpath:subdirs
> > > include_directories: includes,
> > > dependencies: static_deps,
> > > c_args: cflags,
> > > - install: true)
> > > + install: install_static)
> > >
> > > # now build the shared driver
> > > version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> > > @@ -271,55 +276,46 @@ foreach subpath:subdirs
> > > endif
> > > endif
> > >
> > > - if is_windows
> > > - if is_ms_linker
> > > + if is_shared_enabled
> > > + if is_ms_compiler
> > > def_file = custom_target(lib_name + '_def',
> > > command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > > input: version_map,
> > > output: '@0@_exports.def'.format(lib_name))
> > > lk_deps += [def_file]
> > > -
> > > - lk_args = ['-Wl,/def:' + def_file.full_path()]
> > > - if meson.version().version_compare('<0.54.0')
> > > - lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> > > - endif
> > > + lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
> >
> > Again, the /experimental flag shouldn't be necessary, since it's set as
> > project option in config/meson.build.
>
> as per above, let me know if what i said seems crazy.
>
> >
> > > else
> > > - mingw_map = custom_target(lib_name + '_mingw',
> > > - command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > > - input: version_map,
> > > - output: '@0@_mingw.map'.format(lib_name))
> > > - lk_deps += [mingw_map]
> > > -
> > > - lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> > > + lk_args = ['-Wl,--version-script=' + version_map]
> > > endif
> > > - else
> > > - lk_args = ['-Wl,--version-script=' + version_map]
> > > endif
> > >
> > > - shared_lib = shared_library(lib_name, sources,
> > > - objects: objs,
> > > - include_directories: includes,
> > > - dependencies: shared_deps,
> > > - c_args: cflags,
> > > - link_args: lk_args,
> > > - link_depends: lk_deps,
> > > - version: abi_version,
> > > - soversion: so_version,
> > > - install: true,
> > > - install_dir: driver_install_path)
> > > -
> > > - # create a dependency object and add it to the global dictionary so
> > > - # testpmd or other built-in apps can find it if necessary
> > > - shared_dep = declare_dependency(link_with: shared_lib,
> > > - include_directories: includes,
> > > - dependencies: shared_deps)
> > > + if is_shared_enabled
> > > + shared_lib = shared_library(lib_name, sources,
> > > + objects: objs,
> > > + include_directories: includes,
> > > + dependencies: shared_deps,
> > > + c_args: cflags,
> > > + link_args: lk_args,
> > > + link_depends: lk_deps,
> > > + version: abi_version,
> > > + soversion: so_version,
> > > + install: install_shared,
> > > + install_dir: driver_install_path)
> > > +
> > > + # create a dependency object and add it to the global dictionary so
> > > + # testpmd or other built-in apps can find it if necessary
> > > + shared_dep = declare_dependency(link_with: shared_lib,
> > > + include_directories: includes,
> > > + dependencies: shared_deps)
> > > +
> > > + set_variable('shared_@0@'.format(lib_name), shared_dep)
> >
> > Just a thought - could you avoid some of the changes throughout this set,
> > if you add an else-leg to the "is_shared_enabled" check, where, if shared
> > is not enabled, you assign shared_dep to be an empty string, or an empty
> > object or something.
> >
> > That would mean that you wouldn't need to move the set_variable line as
> > here, you also wouldn't need to put conditionals around all the
> >
> > Just a thought - could you avoid some of the changes throughout this set,
> > if you add an else-leg to the "is_shared_enabled" check, where, if shared
> > is not enabled, you assign shared_dep to be an empty string, or an empty
> > object or something.
> >
> > That would mean that you wouldn't need to move the set_variable line as
> > here, you also wouldn't need to put conditionals around all the
> > 'shared_deps +=' lines when processing deps for a component, and lastly,
> > you may not even need to change all the searches for shared_rte_ to
> > static_rte, since the variables would exist for shared - they'd just be
> > empty!
>
> did this in v3, still requires a diff -b to read what actually changed
> but appears to have worked out as you anticipated. so i think probably
> it was a good improvement.
>
Great, thanks for trying it!
> >
> > > + endif
> > > static_dep = declare_dependency(
> > > include_directories: includes,
> > > dependencies: static_deps)
> > >
> > > dpdk_drivers += static_lib
> >
> > <snip for brevity>
@@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
default_ldflags = []
if get_option('default_library') == 'static' and not is_windows
default_ldflags += ['-Wl,--export-dynamic']
+elif get_option('default_library') == 'shared' and is_ms_compiler
+ default_ldflags += ['/experimental:tlsDllInterface']
endif
foreach app:apps
@@ -104,6 +106,10 @@ foreach app:apps
link_libs = dpdk_static_libraries + dpdk_drivers
endif
+ if is_windows and is_shared_enabled
+ cflags += '-DRTE_BUILD_SHARED_LIB'
+ endif
+
exec = executable('dpdk-' + name,
sources,
c_args: cflags,
@@ -53,6 +53,9 @@ if is_ms_compiler
# enable statement expressions extension
add_project_arguments('/experimental:statementExpressions', language: 'c')
+
+ # enable export of thread_local variables
+ add_project_arguments('/experimental:tlsDllInterface', language: 'c')
endif
# set the major version, which might be used by drivers and libraries
@@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
NOTE: DPDK always builds both shared and static libraries. Please set
"default_library" to either "static" or "shared" to select default linkage
for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+ error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+if is_windows
+ if is_ms_compiler and get_option('default_library') == 'shared'
+ is_shared_enabled=true
+ else
+ is_shared_enabled=false
+ endif
+else
+ is_shared_enabled=true
+endif
+
+if is_windows
+ install_static = get_option('default_library') == 'static'
+ install_shared = get_option('default_library') == 'shared'
+else
+ install_static=true
+ install_shared=true
endif
@@ -80,7 +80,7 @@ foreach subpath:subdirs
subdir(class)
skip_class = false
foreach d:std_deps
- if not is_variable('shared_rte_' + d)
+ if not is_variable('static_rte_' + d)
skip_class = true
reason = 'missing internal dependency, "@0@"'.format(d)
if dpdk_libs_deprecated.contains(d)
@@ -173,7 +173,7 @@ foreach subpath:subdirs
if not build
break
endif
- if not is_variable('shared_rte_' + d)
+ if not is_variable('static_rte_' + d)
build = false
reason = 'missing internal dependency, "@0@"'.format(d)
if dpdk_libs_deprecated.contains(d)
@@ -188,7 +188,9 @@ foreach subpath:subdirs
+'\tPlease enable missing dependency "@0@"'.format(d))
endif
else
- shared_deps += [get_variable('shared_rte_' + d)]
+ if is_shared_enabled
+ shared_deps += [get_variable('shared_rte_' + d)]
+ endif
static_deps += [get_variable('static_rte_' + d)]
endif
endforeach
@@ -208,6 +210,9 @@ foreach subpath:subdirs
enabled_drivers += name
lib_name = '_'.join(['rte', class, name])
cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
+ if is_windows and is_shared_enabled
+ cflags += '-DRTE_BUILD_SHARED_LIB'
+ endif
if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
cflags += '-DRTE_ANNOTATE_LOCKS'
cflags += '-Wthread-safety'
@@ -247,7 +252,7 @@ foreach subpath:subdirs
include_directories: includes,
dependencies: static_deps,
c_args: cflags,
- install: true)
+ install: install_static)
# now build the shared driver
version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,55 +276,46 @@ foreach subpath:subdirs
endif
endif
- if is_windows
- if is_ms_linker
+ if is_shared_enabled
+ if is_ms_compiler
def_file = custom_target(lib_name + '_def',
command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
input: version_map,
output: '@0@_exports.def'.format(lib_name))
lk_deps += [def_file]
-
- lk_args = ['-Wl,/def:' + def_file.full_path()]
- if meson.version().version_compare('<0.54.0')
- lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
- endif
+ lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
else
- mingw_map = custom_target(lib_name + '_mingw',
- command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
- input: version_map,
- output: '@0@_mingw.map'.format(lib_name))
- lk_deps += [mingw_map]
-
- lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+ lk_args = ['-Wl,--version-script=' + version_map]
endif
- else
- lk_args = ['-Wl,--version-script=' + version_map]
endif
- shared_lib = shared_library(lib_name, sources,
- objects: objs,
- include_directories: includes,
- dependencies: shared_deps,
- c_args: cflags,
- link_args: lk_args,
- link_depends: lk_deps,
- version: abi_version,
- soversion: so_version,
- install: true,
- install_dir: driver_install_path)
-
- # create a dependency object and add it to the global dictionary so
- # testpmd or other built-in apps can find it if necessary
- shared_dep = declare_dependency(link_with: shared_lib,
- include_directories: includes,
- dependencies: shared_deps)
+ if is_shared_enabled
+ shared_lib = shared_library(lib_name, sources,
+ objects: objs,
+ include_directories: includes,
+ dependencies: shared_deps,
+ c_args: cflags,
+ link_args: lk_args,
+ link_depends: lk_deps,
+ version: abi_version,
+ soversion: so_version,
+ install: install_shared,
+ install_dir: driver_install_path)
+
+ # create a dependency object and add it to the global dictionary so
+ # testpmd or other built-in apps can find it if necessary
+ shared_dep = declare_dependency(link_with: shared_lib,
+ include_directories: includes,
+ dependencies: shared_deps)
+
+ set_variable('shared_@0@'.format(lib_name), shared_dep)
+ endif
static_dep = declare_dependency(
include_directories: includes,
dependencies: static_deps)
dpdk_drivers += static_lib
- set_variable('shared_@0@'.format(lib_name), shared_dep)
set_variable('static_@0@'.format(lib_name), static_dep)
# for drivers, we only need to add dependency objects for static libs,
# shared lib drivers are not linked in
@@ -10,7 +10,7 @@ sources = [
depends = ['ethdev', 'mempool_octeontx']
static_objs = []
foreach d: depends
- if not is_variable('shared_rte_' + d)
+ if not is_variable('static_rte_' + d)
subdir_done()
endif
static_objs += get_variable('static_rte_' + d)
@@ -85,6 +85,8 @@ endif
default_ldflags = dpdk_extra_ldflags
if get_option('default_library') == 'static' and not is_windows
default_ldflags += ['-Wl,--export-dynamic']
+elif is_ms_compiler
+ default_ldflags += ['/experimental:tlsDllInterface']
endif
foreach example: examples
@@ -121,6 +123,10 @@ foreach example: examples
continue
endif
+ if is_windows and is_shared_enabled
+ cflags += '-DRTE_BUILD_SHARED_LIB'
+ endif
+
if allow_experimental_apis
cflags += '-DALLOW_EXPERIMENTAL_API'
endif
@@ -184,7 +184,7 @@ foreach l:libraries
if not build
break
endif
- if not is_variable('shared_rte_' + d)
+ if not is_variable('static_rte_' + d)
build = false
reason = 'missing internal dependency, "@0@"'.format(d)
if dpdk_libs_deprecated.contains(d)
@@ -197,7 +197,9 @@ foreach l:libraries
+ '\tPlease add missing dependency "@0@" to "enable_libs" option'.format(d))
endif
else
- shared_deps += [get_variable('shared_rte_' + d)]
+ if is_shared_enabled
+ shared_deps += [get_variable('shared_rte_' + d)]
+ endif
static_deps += [get_variable('static_rte_' + d)]
endif
endforeach
@@ -235,6 +237,10 @@ foreach l:libraries
cflags += '-Wthread-safety'
endif
+ if is_windows and is_shared_enabled
+ cflags += '-DRTE_BUILD_SHARED_LIB'
+ endif
+
# first build static lib
static_lib = static_library(libname,
sources,
@@ -242,7 +248,7 @@ foreach l:libraries
c_args: cflags,
dependencies: static_deps,
include_directories: includes,
- install: true)
+ install: install_static)
static_dep = declare_dependency(
include_directories: includes,
dependencies: static_deps)
@@ -260,33 +266,14 @@ foreach l:libraries
version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), l)
lk_deps = [version_map]
- if is_ms_linker
- def_file = custom_target(libname + '_def',
- command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
- input: version_map,
- output: '@0@_exports.def'.format(libname))
- lk_deps += [def_file]
-
+ if is_shared_enabled
if is_ms_compiler
- lk_args = ['/def:' + def_file.full_path()]
- if meson.version().version_compare('<0.54.0')
- lk_args += ['/implib:lib\\librte_' + l + '.dll.a']
- endif
- else
- lk_args = ['-Wl,/def:' + def_file.full_path()]
- if meson.version().version_compare('<0.54.0')
- lk_args += ['-Wl,/implib:lib\\librte_' + l + '.dll.a']
- endif
- endif
- else
- if is_windows
- mingw_map = custom_target(libname + '_mingw',
+ def_file = custom_target(libname + '_def',
command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
input: version_map,
- output: '@0@_mingw.map'.format(libname))
- lk_deps += [mingw_map]
-
- lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+ output: '@0@_exports.def'.format(libname))
+ lk_deps += [def_file]
+ lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
else
lk_args = ['-Wl,--version-script=' + version_map]
endif
@@ -304,27 +291,29 @@ foreach l:libraries
output: name + '.sym_chk')
endif
- shared_lib = shared_library(libname,
- sources,
- objects: objs,
- c_args: cflags,
- dependencies: shared_deps,
- include_directories: includes,
- link_args: lk_args,
- link_depends: lk_deps,
- version: abi_version,
- soversion: so_version,
- install: true)
- shared_dep = declare_dependency(link_with: shared_lib,
- include_directories: includes,
- dependencies: shared_deps)
+ if is_shared_enabled
+ shared_lib = shared_library(libname,
+ sources,
+ objects: objs,
+ c_args: cflags,
+ dependencies: shared_deps,
+ include_directories: includes,
+ link_args: lk_args,
+ link_depends: lk_deps,
+ version: abi_version,
+ soversion: so_version,
+ install: install_shared)
+ shared_dep = declare_dependency(link_with: shared_lib,
+ include_directories: includes,
+ dependencies: shared_deps)
- dpdk_libraries = [shared_lib] + dpdk_libraries
- dpdk_static_libraries = [static_lib] + dpdk_static_libraries
+ dpdk_libraries = [shared_lib] + dpdk_libraries
+ set_variable('shared_rte_' + name, shared_dep)
+ dpdk_shared_lib_deps += shared_dep
+ endif
- set_variable('shared_rte_' + name, shared_dep)
+ dpdk_static_libraries = [static_lib] + dpdk_static_libraries
set_variable('static_rte_' + name, static_dep)
- dpdk_shared_lib_deps += shared_dep
dpdk_static_lib_deps += static_dep
if developer_mode
message('lib/@0@: Defining dependency "@1@"'.format(l, name))