[v2,00/10] Add an option to use LTO for DPDK build

  This patch series adds an option to make use of link time optimization
(if compiler has support for it).  It is split as follows:
- 1st patch (build) is the enablement
- remaining patches are fixes for the warnings produced by the compiler
  and they are split by directory/subsystem so their maintainers can
  easily find and verify the changes - please note that there are two
  groups:
  * errors (or possible errors) - with title "fix possible use ..."
  * false positives - warnings that _I_ think are not valid and the
    changes are made only to silence the compiler.

v2 Changes:
-----------
- fixed building of shared libraries:
  - gcc does not scan top level assembler statements so it missed that
    function implementations for specific versions were being used and
    was removing them
- fixed meson.build config files:
  - moved from 'enable_lto' project option to built-in 'b_lto'
  - documented that 'default_library' should be 'shared':
    with 'default_library=static' (the default) the meson build with LTO
    is broken - this is because, SHARED_LIB is fixed in rte_config.h
    used by meson which leads to no alias for default version in
    generated static libraries (MAP_STATIC_SYMBOL() is empty)
- app/test: added log for failed bonding "config get"

Andrzej Ostruszka (10):
  build: add an option to enable LTO build
  eventdev: fix possible use of uninitialized var
  app/eventdev: fix maybe-uninitialized warnings for LTO build
  event/octeontx2: fix maybe-uninitialized warnings for LTO build
  app/test: fix maybe-uninitialized warnings for LTO build
  net/dpaa2: fix possible use of uninitialized vars
  net/e1000: fix maybe-uninitialized warnings for LTO build
  net/i40e: fix maybe-uninitialized warnings for LTO build
  net/ifc: fix maybe-uninitialized warnings for LTO build
  net/qede: fix maybe-uninitialized warnings for LTO build

 .travis.yml                                   |  7 ++++
 app/test-eventdev/test_perf_common.c          |  2 +-
 app/test-eventdev/test_pipeline_common.c      |  4 +-
 app/test/test_hash_readwrite.c                |  2 +-
 app/test/test_link_bonding_mode4.c            | 10 ++++-
 app/test/test_memzone.c                       |  3 +-
 config/common_base                            |  5 +++
 config/meson.build                            | 15 ++++++++
 doc/guides/prog_guide/lto.rst                 | 37 +++++++++++++++++++
 doc/guides/rel_notes/release_19_11.rst        |  8 ++++
 drivers/event/octeontx2/otx2_tim_worker.h     |  2 +-
 drivers/net/dpaa2/base/dpaa2_hw_dpni.c        |  1 +
 drivers/net/dpaa2/mc/dpkg.c                   |  2 +-
 drivers/net/dpaa2/mc/dpni.c                   |  9 +++--
 drivers/net/e1000/base/e1000_82543.c          |  2 +-
 drivers/net/e1000/base/e1000_ich8lan.c        |  2 +-
 drivers/net/e1000/base/e1000_phy.c            |  2 +-
 drivers/net/i40e/i40e_ethdev.c                |  2 +-
 drivers/net/ifc/ifcvf_vdpa.c                  | 14 ++++---
 drivers/net/qede/base/ecore_mcp.c             | 13 ++++---
 lib/librte_distributor/rte_distributor.c      | 18 ++++-----
 lib/librte_distributor/rte_distributor_v20.c  | 18 ++++-----
 lib/librte_eventdev/rte_event_timer_adapter.c |  8 ++--
 lib/librte_lpm/rte_lpm.c                      | 28 +++++++-------
 lib/librte_lpm/rte_lpm6.c                     | 16 ++++----
 lib/librte_timer/rte_timer.c                  | 20 +++++-----
 mk/toolchain/clang/rte.toolchain-compat.mk    |  4 ++
 mk/toolchain/clang/rte.vars.mk                |  8 ++++
 mk/toolchain/gcc/rte.toolchain-compat.mk      |  4 ++
 mk/toolchain/gcc/rte.vars.mk                  | 12 ++++++
 mk/toolchain/icc/rte.vars.mk                  |  8 ++++
 31 files changed, 205 insertions(+), 81 deletions(-)
 create mode 100644 doc/guides/prog_guide/lto.rst
  

On Mon, Oct 21, 2019 at 12:56:58PM +0200, Andrzej Ostruszka wrote:
> This patch adds an option to enable link time optimization.  In addition
> to LTO option itself (-flto) fat-lto-objects are being used.  This is
> because during the build pmdinfogen scans the generated ELF objects to
> find this_pmd_name* symbol in symbol table.  Without fat-lto-objects gcc
> produces ELF only with extra symbols for internal use during linking.
> 
> Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
> ---
>  .travis.yml                                  |  7 ++++
>  config/common_base                           |  5 +++
>  config/meson.build                           | 15 ++++++++
>  doc/guides/prog_guide/lto.rst                | 36 ++++++++++++++++++++
>  doc/guides/rel_notes/release_19_11.rst       |  8 +++++
>  lib/librte_distributor/rte_distributor.c     | 18 +++++-----
>  lib/librte_distributor/rte_distributor_v20.c | 18 +++++-----
>  lib/librte_lpm/rte_lpm.c                     | 28 +++++++--------
>  lib/librte_lpm/rte_lpm6.c                    | 16 ++++-----
>  lib/librte_timer/rte_timer.c                 | 20 +++++------
>  mk/toolchain/gcc/rte.toolchain-compat.mk     |  4 +++
>  mk/toolchain/gcc/rte.vars.mk                 | 12 +++++++
>  mk/toolchain/icc/rte.vars.mk                 |  8 +++++
>  13 files changed, 145 insertions(+), 50 deletions(-)
>  create mode 100644 doc/guides/prog_guide/lto.rst
> 
> diff --git a/.travis.yml b/.travis.yml
> index 781f9f666..70d221852 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -31,6 +31,7 @@ env:
>    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
>    - DEF_LIB="shared" RUN_TESTS=1 BUILD_DOCS=1
> +  - DEF_LIB="shared" OPTS="-Db_lto=true"
>  
>  matrix:
>    include:
> @@ -100,6 +101,12 @@ matrix:
>        apt:
>          packages:
>            - *extra_packages
> +  - env: DEF_LIB="shared" OPTS="-Db_lto=true" EXTRA_PACKAGES=1
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - *extra_packages
>  
>  
>  script: ./.ci/${TRAVIS_OS_NAME}-build.sh
> diff --git a/config/common_base b/config/common_base
> index 8ef75c203..73a55fdec 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -49,6 +49,11 @@ CONFIG_RTE_FORCE_INTRINSICS=n
>  #
>  CONFIG_RTE_ARCH_STRICT_ALIGN=n
>  
> +#
> +# Enable link time optimization
> +#
> +CONFIG_RTE_ENABLE_LTO=n
> +
>  #
>  # Compile to share library
>  #
> diff --git a/config/meson.build b/config/meson.build
> index 2bafea530..97bbc323b 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -196,3 +196,18 @@ add_project_arguments('-D_GNU_SOURCE', language: 'c')
>  if is_freebsd
>  	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
>  endif
> +
> +if get_option('b_lto')
> +	if cc.has_argument('-ffat-lto-objects')
> +		add_project_arguments('-ffat-lto-objects', language: 'c')
> +	else
> +		error('compiler does not support fat LTO objects - please turn LTO off')
> +	endif
> +	if cc.get_id() == 'gcc'
> +		# workaround for bug 81440
> +		if cc.version().version_compare('<8.0')
> +			add_project_arguments('-Wno-lto-type-mismatch', language: 'c')
> +			add_project_link_arguments('-Wno-lto-type-mismatch', language: 'c')
> +		endif
> +	endif
> +endif

Just wondering on this what would be the impact with blindly disabling this
warning? Would we miss any common issues accidentally?

If we keep the current conditional code, suggest we merge the two checks
for gcc and gcc version together into one if statement so we can reduce
indentation level by one.

/Bruce
  

Thank you Bruce for the comment.  The original patch set did not manage
to get to the dev list (I've sent it from my Marvell account and it
stuck at moderation).  So I'll take your "indent" comment and will send
another version which will hopefully get through.

On 10/21/19 2:59 PM, Bruce Richardson wrote:
[...]
>> diff --git a/config/meson.build b/config/meson.build
>> index 2bafea530..97bbc323b 100644
>> --- a/config/meson.build
>> +++ b/config/meson.build
>> @@ -196,3 +196,18 @@ add_project_arguments('-D_GNU_SOURCE', language: 'c')
>>  if is_freebsd
>>  	add_project_arguments('-D__BSD_VISIBLE', language: 'c')
>>  endif
>> +
>> +if get_option('b_lto')
>> +	if cc.has_argument('-ffat-lto-objects')
>> +		add_project_arguments('-ffat-lto-objects', language: 'c')
>> +	else
>> +		error('compiler does not support fat LTO objects - please turn LTO off')
>> +	endif
>> +	if cc.get_id() == 'gcc'
>> +		# workaround for bug 81440
>> +		if cc.version().version_compare('<8.0')
>> +			add_project_arguments('-Wno-lto-type-mismatch', language: 'c')
>> +			add_project_link_arguments('-Wno-lto-type-mismatch', language: 'c')
>> +		endif
>> +	endif
>> +endif
> 
> Just wondering on this what would be the impact with blindly disabling this
> warning? Would we miss any common issues accidentally?

This warning is for the cases where declarations (of e.g. global
variable) have different types in different compilation units - these
type of things can only be caught with LTO.  I don't think we need be
worrying about that in DPDK - the problem is a bug in gcc that produces
these warnings for the case of structs with flexible arrays at the end
(which are fairly common in DPDK).

> If we keep the current conditional code, suggest we merge the two checks
> for gcc and gcc version together into one if statement so we can reduce
> indentation level by one.

Agree.  Will send another version - which hopefully will get through to
the dev list.

Regards
Andrzej
  

On a relate topic.

Last time, tried build a DPDK application using -fwhole-program gcc gave
lots of warnings because it decided not to inline rte_memcpy.

Perhaps this might impact LTO as well. Really rte_memcpy_func should not
be inline. We already optimize for the constant size case where inline
makes sense. After that not so much.