Re: [dpdk-dev] [PATCH v2 01/10] build: add an option to enable LTO build

[Neil Horman <nhorman@tuxdriver.com>]

On Tue, Sep 24, 2019 at 12:25:35PM +0200, Bruce Richardson wrote:
> On Tue, Sep 24, 2019 at 08:46:25AM +0200, Andrzej Ostruszka wrote:
> > On 9/23/19 6:13 PM, Bruce Richardson wrote:
> > [...]
> > > However, testing on my system with the meson build, I'm getting lots of
> > > link errors [See below]. Any suggestions?
> > > 
> > > /Bruce
> > > 
> > > /usr/bin/ld: /tmp/dpdk-testpmd.hncbtE.ltrans93.ltrans.o: in function `ena_stop':
> > > <artificial>:(.text+0x9ed6): undefined reference to `rte_timer_stop'
> > [...]
> > 
> > What is 'default_library'?  It should be 'shared' as mentioned in the
> > docs.  The problem is that RTE_BUILD_SHARED_LIB is statically defined in
> > rte_config.h used by meson build.  This results in broken static
> > libraries (those that are using versioned symbols - like
> > timer/lpm/distributor) - since the MAP_STATIC_SYMBOL macro defining the
> > default alias is empty.  With LTO compiler (or rather linker) is quite
> > aggressive in removing stuff that it thinks is not needed.
> > 
> > Regards
> > Andrzej
> > 
> > PS. IMHO this SHARED_LIB define should be removed from the rte_config.h
> > and meson.build should be updated to detect 'default_library' and add it
> > as needed.  Don't know exactly how meson behaves if 'default_library' is
> > 'both' - the docs say that it reuses objects from static build, so we
> > might have to work around it for LTO & 'both'.
> 
> That proposal won't work either as we build both static and shared
> libraries in all cases - the default_library value only affects whether the
> test apps are linked against the .a or .so files.
> 
> The real issue seems to be that the compat.h header has different
> compilation paths for static and shared libraries, which means that any C
> file including it can't have a .o file that can be used for both a .a and a
> .so simultaneously. Having it default to the shared library path seems to
> work fine thus far, but with LTO it seems broken as you say. Adding Neil as
> the original file author of this in case he has any suggestions here. I'd
> really rather not have to go back to building .a's and .so's separately.
> 
The notion of using the same object file to link to a static archive and a dso
seems somewhat suspect to me in general.  I say that because I don't see a way
for the linker to know/prove at link time that the options used to compile an
object for target (a) will be the same as those used to compile the same object
for target (b).  In this particular case, you've identified an issue in
compilation changes that triggers off the building of dso's vs static archives,
but I could envision a scenario in which you might try to build targets for BSD
and Linux in parallel, or even for different machines (i.e. build for a least
common denominator x8664 target, and a highly optimized recent x8664 processor
with all the ISA extensions enabled). We don't do that currently now of course,
but we could, and the only way we could do so would be to rebuild all the
objects with the compilation flags for each separately.

That said, if the goal is to just overcome this particular situation, it might
(strong might), be sufficient to simply augment the MAP_STATIC_SYMBOL macro in
the CONFIG_RTE_BUILD_SHARED_LIB=n case to append the 'used' attribute.
Ostensibly, LTO would be smart enough then to not eliminate the symbol?  Just a
thought.

But I think we need to take care here.  While its fine solve this particular
situation, I think the notion of reusing objects for multiple link targets has
the potential to uncover many issues of this class, which won't be as solveable
without having to just rebuild objects from scratch.

Neil

> Regards,
> /Bruce
>