[v6,00/12] 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 is just a minor doc fix for versioning macros
- 2nd patch fixes missing __vsym annotations (needed for LTO)
- 3rd is the LTO 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.

v6 Changes:
-----------
- yet another split of the patch: this time from __vsym one minor doc
  fix to versioning macros has been moved to another patch
- net/ifc - fixed initialization of vring in m_ifcvf_start():
  after rebase definition rte_vhost_vring has changed and now simple
  '{ 0 }' produces proper warning - however instead of '{ { 0 } }'
  I went with memset() because I could not get gcc to be quiet about
  "missing initializer" (IMHO "{{0}}" is a correct initialization but
  gcc required me to spell out initializers to all members which looked
  ugly and would require update on further changes to vring)
- I've been suggested (offlist) to mark the patches that were previously
  "reviewed/acked" so I've added some "Reviewed|Acked-by" tags (if the
  ack was sent to a patch that was later split the tag is placed in
  both patches).


v5 Changes:
-----------
- rebased to the v19.11 (one additional warning silenced in net/qede)
- initial patch has been split into two:
  - one adding '__vsym' annotations and clarifying versioning docs
  - another adding the actual LTO enablement
- addressed various documentation flaws pointed during review

v4 Changes:
-----------
- merge nested conditionals in config/meson.build into one

v3 Changes:
-----------
- removed support for clang (only gcc and icc remain):
  - it turned out that 'fat-lto-objects' is not really supported - it is
    accepted as a flag but ignored e.g. clang v6.0:
      clang: error: optimization flag '-ffat-lto-objects' is not supported
      [-Werror,-Wignored-optimization-argument]
    this was probably masked previously by playing around with meson and
    testing for the availability of this flag and dropping LTO if not
    supported.
- improved commit messages:
  - added 'Fixes' tags where appropriate
  - included snippets of compiler warnings
- changed commit names for the commits cleaning up 'false positive'
  compiler warnings - these are not really 'fixes' for the code, just
  workarounds for compiler shortcomings and check-git-log.sh was
  complaining about missing 'Fixes' tag.

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 (12):
  doc: fix description of versioning macros
  build: annotate versioned symbols with __vsym macro
  build: add an option to enable LTO build
  eventdev: fix possible use of uninitialized var
  app/eventdev: clean LTO build warnings (maybe-uninitialized)
  event/octeontx2: clean LTO build warnings (maybe-uninitialized)
  app/test: clean LTO build warnings (maybe-uninitialized)
  net/dpaa2: fix possible use of uninitialized vars
  net/e1000: clean LTO build warnings (maybe-uninitialized)
  net/i40e: clean LTO build warnings (maybe-uninitialized)
  net/ifc: clean LTO build warnings (maybe-uninitialized)
  net/qede: clean LTO build warnings (maybe-uninitialized)

 .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            |  8 +++-
 app/test/test_memzone.c                       |  3 +-
 config/common_base                            |  5 +++
 config/meson.build                            | 13 ++++++
 doc/guides/contributing/versioning.rst        | 18 ++++++---
 doc/guides/prog_guide/index.rst               |  1 +
 doc/guides/prog_guide/lto.rst                 | 40 +++++++++++++++++++
 doc/guides/rel_notes/release_19_11.rst        |  9 +++++
 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             | 15 +++----
 lib/librte_distributor/rte_distributor.c      | 18 ++++-----
 lib/librte_distributor/rte_distributor_v20.c  | 18 ++++-----
 .../common/include/rte_function_versioning.h  | 11 ++++-
 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/gcc/rte.toolchain-compat.mk      |  4 ++
 mk/toolchain/gcc/rte.vars.mk                  | 12 ++++++
 mk/toolchain/icc/rte.vars.mk                  |  8 ++++
 32 files changed, 218 insertions(+), 88 deletions(-)
 create mode 100644 doc/guides/prog_guide/lto.rst
  

On 10/29/19 3:12 PM, Andrzej Ostruszka wrote:
> This patch series adds an option to make use of link time optimization
> (if compiler has support for it).
[...]
>  .travis.yml                                   |  7 ++++
[...]

Aaron, Michael, all

I'd probably need some assistance with Travis.  This patchset added
following changes:

diff --git a/.travis.yml b/.travis.yml
index 3d6ef2959..3cd746dba 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -34,6 +34,7 @@ env:
   - DEF_LIB="static" OPTS="-Denable_kmods=false"
   - DEF_LIB="shared" OPTS="-Denable_kmods=false"
   - DEF_LIB="shared" RUN_TESTS=1
+  - DEF_LIB="shared" OPTS="-Db_lto=true"

 matrix:
   include:
@@ -105,6 +106,12 @@ matrix:
       apt:
         packages:
           - *extra_packages
+  - env: DEF_LIB="shared" OPTS="-Db_lto=true" EXTRA_PACKAGES=1
+    compiler: gcc
+    addons:
+      apt:
+        packages:
+          - *extra_packages


however I have to admit I'm not familiar with Travis and the actual
meaning of that configuration (that is it's actual mapping to builds).
I'm getting following CI errors:

1. https://travis-ci.com/ovsrobot/dpdk/jobs/250599578

This is with clang - this patchset should not be run with clang as it is
not supported.  In build "matrix" there is 'gcc' specified but somehow
clang build is attempted.  Please advice me how to change this
.travis.yml to stop that - should I remove entry in "env"?

2. https://travis-ci.com/ovsrobot/dpdk/jobs/250599577
   https://travis-ci.com/ovsrobot/dpdk/jobs/250599591

This is with gcc and meson with shared library and I can't reproduce
this.  The are two problems reported, first a compiler warning which
seems to be not connected at all with the changes:

--8<------------------------
/usr/include/x86_64-linux-gnu/bits/unistd.h:39:9: warning: call to
‘__read_chk_warn’ declared with attribute warning: read called with
bigger length than size of the destination buffer
  return __read_chk (__fd, __buf, __nbytes, __bos0 (__buf));
         ^
In function ‘__read_alias’,
    inlined from ‘eal_intr_process_interrupts’ at
../lib/librte_eal/linux/eal/eal_interrupts.c:911:17,
    inlined from ‘eal_intr_handle_interrupts’ at
../lib/librte_eal/linux/eal/eal_interrupts.c:1030:7,
    inlined from ‘eal_intr_thread_main’ at
../lib/librte_eal/linux/eal/eal_interrupts.c:1100:3:
--8<------------------------

and a second linker error:

--8<------------------------
FAILED: gcc  -o lib/librte_lpm.so.2.1
'lib/76b5a35@@rte_lpm@sha/librte_lpm_rte_lpm.c.o'
'lib/76b5a35@@rte_lpm@sha/librte_lpm_rte_lpm6.c.o' -flto -Wl,--as-needed
-Wl,--no-undefined -Wl,-O1 -shared -fPIC -Wl,--start-group
-Wl,-soname,librte_lpm.so.2 -Wl,--no-as-needed -pthread -lm -ldl -lnuma
-Wno-lto-type-mismatch lib/librte_eal.so.12.1 lib/librte_kvargs.so.1.1
lib/librte_hash.so.2.1 lib/librte_ring.so.2.1
-Wl,--version-script=/home/travis/build/ovsrobot/dpdk/lib/librte_lpm/rte_lpm_version.map
/usr/lib/x86_64-linux-gnu/libbsd.so -Wl,--end-group
'-Wl,-rpath,$ORIGIN/'
-Wl,-rpath-link,/home/travis/build/ovsrobot/dpdk/build/lib
/tmp/ccZPEQKf.ltrans3.ltrans.o: In function `rte_lpm6_delete_bulk_func':
<artificial>:(.text+0xf01): undefined reference to `rte_lpm6_add'
--8<------------------------

As I've said I can't reproduce none of these problems - are they some CI
issues?

I'd appreciate some help with these.

Regards
Andrzej
  

Andrzej Ostruszka <amo@semihalf.com> writes:

> On 10/29/19 3:12 PM, Andrzej Ostruszka wrote:
>> This patch series adds an option to make use of link time optimization
>> (if compiler has support for it).
> [...]
>>  .travis.yml                                   |  7 ++++
> [...]
>
> Aaron, Michael, all
>
> I'd probably need some assistance with Travis.  This patchset added
> following changes:
>
> diff --git a/.travis.yml b/.travis.yml
> index 3d6ef2959..3cd746dba 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -34,6 +34,7 @@ env:
>    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
>    - DEF_LIB="shared" RUN_TESTS=1
> +  - DEF_LIB="shared" OPTS="-Db_lto=true"
>
>  matrix:
>    include:
> @@ -105,6 +106,12 @@ matrix:
>        apt:
>          packages:
>            - *extra_packages
> +  - env: DEF_LIB="shared" OPTS="-Db_lto=true" EXTRA_PACKAGES=1
> +    compiler: gcc
> +    addons:
> +      apt:
> +        packages:
> +          - *extra_packages
>
>
> however I have to admit I'm not familiar with Travis and the actual
> meaning of that configuration (that is it's actual mapping to builds).
> I'm getting following CI errors:
>
> 1. https://travis-ci.com/ovsrobot/dpdk/jobs/250599578
>
> This is with clang - this patchset should not be run with clang as it is
> not supported.  In build "matrix" there is 'gcc' specified but somehow
> clang build is attempted.  Please advice me how to change this
> .travis.yml to stop that - should I remove entry in "env"?

Yes, do not use the non-matrix defines.

IE: drop the hunk '@@ -34,6 +34,7 @@ env:'

The other hunk '@@ -105,6 +106,12 @@ matrix:' is sufficient.
You might want a secondary copy that includes AARCH64=1 if fat LTO is
supported under ARM64 (assuming there's a benefit to doing this build).

> 2. https://travis-ci.com/ovsrobot/dpdk/jobs/250599577
>    https://travis-ci.com/ovsrobot/dpdk/jobs/250599591
>
> This is with gcc and meson with shared library and I can't reproduce
> this.  The are two problems reported, first a compiler warning which
> seems to be not connected at all with the changes:

These jobs are running on ubuntu xenial, using gcc 5.4.0 compiler.

Are you sure that you've passed all of the requisite linker flags for
this version of gcc?

Alternatively, you can make a package list that will include a version
of the compiler you expect to support.  But that should also be
reflected in the documentation.  I didn't see any compiler restrictions
documented.

> --8<------------------------
> /usr/include/x86_64-linux-gnu/bits/unistd.h:39:9: warning: call to
> ‘__read_chk_warn’ declared with attribute warning: read called with
> bigger length than size of the destination buffer
>   return __read_chk (__fd, __buf, __nbytes, __bos0 (__buf));
>          ^
> In function ‘__read_alias’,
>     inlined from ‘eal_intr_process_interrupts’ at
> ../lib/librte_eal/linux/eal/eal_interrupts.c:911:17,
>     inlined from ‘eal_intr_handle_interrupts’ at
> ../lib/librte_eal/linux/eal/eal_interrupts.c:1030:7,
>     inlined from ‘eal_intr_thread_main’ at
> ../lib/librte_eal/linux/eal/eal_interrupts.c:1100:3:
> --8<------------------------
>
> and a second linker error:
>
> --8<------------------------
> FAILED: gcc  -o lib/librte_lpm.so.2.1
> 'lib/76b5a35@@rte_lpm@sha/librte_lpm_rte_lpm.c.o'
> 'lib/76b5a35@@rte_lpm@sha/librte_lpm_rte_lpm6.c.o' -flto -Wl,--as-needed
> -Wl,--no-undefined -Wl,-O1 -shared -fPIC -Wl,--start-group
> -Wl,-soname,librte_lpm.so.2 -Wl,--no-as-needed -pthread -lm -ldl -lnuma
> -Wno-lto-type-mismatch lib/librte_eal.so.12.1 lib/librte_kvargs.so.1.1
> lib/librte_hash.so.2.1 lib/librte_ring.so.2.1
> -Wl,--version-script=/home/travis/build/ovsrobot/dpdk/lib/librte_lpm/rte_lpm_version.map
> /usr/lib/x86_64-linux-gnu/libbsd.so -Wl,--end-group
> '-Wl,-rpath,$ORIGIN/'
> -Wl,-rpath-link,/home/travis/build/ovsrobot/dpdk/build/lib
> /tmp/ccZPEQKf.ltrans3.ltrans.o: In function `rte_lpm6_delete_bulk_func':
> <artificial>:(.text+0xf01): undefined reference to `rte_lpm6_add'
> --8<------------------------
>
> As I've said I can't reproduce none of these problems - are they some CI
> issues?

Can you try to reproduce this with a stock xenial image?  If I get the
chance, I will do the same.

> I'd appreciate some help with these.
>
> Regards
> Andrzej