Re: [PATCH v2 3/5] eal: update versioning macros

[Andre Muezerie <andremue@linux.microsoft.com>]

On Thu, Mar 06, 2025 at 01:59:34PM +0100, David Marchand wrote:
> On Wed, Feb 19, 2025 at 2:40 AM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > When compiling with MSVC the error below shows up due to function
> > versioning:
> >
> > ../lib/net/rte_net_crc.c(418): error C2061: syntax error:
> >     identifier '__attribute__'
> >
> > MSVC allows alias function names to be exported, but the mechanism is
> > different than the one used by gcc. It was considered to enhance the logic
> > in the existing version.map files but that file is also passed to other
> > tools on Linux, making this challenging. A simpler approach is to have an
> > optional version.map file to be used only when Microsoft's linker is to be
> > used. This optional map file is only necessary for libraries that have
> > versioned code.
> >
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  lib/eal/include/rte_function_versioning.h | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/lib/eal/include/rte_function_versioning.h b/lib/eal/include/rte_function_versioning.h
> > index eb6dd2bc17..aa3165a659 100644
> > --- a/lib/eal/include/rte_function_versioning.h
> > +++ b/lib/eal/include/rte_function_versioning.h
> > @@ -82,6 +82,13 @@
> >   */
> >  #define MAP_STATIC_SYMBOL(f, p)
> >
> > +/*
> > + * MAP_STATIC_BASE_SYMBOL
> > + * Has the same purpose as MAP_STATIC_SYMBOL, but takes a base function name
> > + * instead of the whole function prototype. It is used to support MSVC.
> > + */
> > +#define MAP_STATIC_BASE_SYMBOL(b, p)
> > +
> >  #else
> >  /*
> >   * No symbol versioning in use
> > @@ -90,7 +97,19 @@
> >  #define VERSION_SYMBOL_EXPERIMENTAL(b, e)
> >  #define __vsym
> >  #define BIND_DEFAULT_SYMBOL(b, e, n)
> > +
> > +#ifdef RTE_TOOLCHAIN_MSVC
> > +#define MAP_STATIC_SYMBOL(f, p)
> 
> I would prefer we trigger an explicit error when some code calling
> MAP_STATIC_SYMBOL() is built with MSVC.
> 
> 
> > +#define MAP_STATIC_BASE_SYMBOL(b, p) __pragma(comment(linker, "/alternatename:" #b "=" #p))
> > +/*
> > + * version.map file should also be updated with "b=p;", like
> > + *     rte_net_crc_set_alg=rte_net_crc_set_alg_v26;
> > + */
> > +#else
> >  #define MAP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p))))
> > +#define MAP_STATIC_BASE_SYMBOL(b, p)
> > +#endif
> > +
> 
> But in any case, I don't like as we end up with one macro for each linker.
> 
> I have been thinking about this topic, and I sent a RFC series, please
> have a look.
> https://patchwork.dpdk.org/project/dpdk/list/?series=34798
> 
> I hope this type of approach will also make it possible to stop
> converting from GNU linker type version.map commited in DPDK to
> Windows linker format.
> 

I like what you're proposing. This makes DPDK less coupled with the GNU linker and
therefore easier to be supported by other toolsets.

> 
> -- 
> David Marchand