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

[Thomas Monjalon <thomas@monjalon.net>]

Adding John and Marko for doc review.

22/10/2019 13:54, Andrzej Ostruszka:
> --- /dev/null
> +++ b/doc/guides/prog_guide/lto.rst

This new file is not included in the prog guide index.

> @@ -0,0 +1,36 @@

You are missing the mandatory SPDX and Copyright header.

> +Link Time Optimization
> +======================
> +
> +The DPDK framework supports compilation with link time optimization

Instead of "The DPDK framework", I prefer just "DPDK".

> +turned on.  This depends obviously on the capabilities of the compiler

Not sure what you are talking about.
If it is about fat objects, it is not obvious.

> +to do "whole program" optimization at link time and is available only
> +for compilers that support that feature (gcc and icc).  To be more

Please start a new sentence at the beginning of a line in the RsT file.
It is easier to review and update.

> +specific compiler have to support creation of ELF objects containing

A comma is missing after "specific".

> +both normal code and internal representation (fat-lto-objects).  This is
> +required since during build some code is generated by parsing produced
> +ELF objects (pmdinfogen).
> +
> +The amount of performance gain that one can get from LTO depends on the
> +compiler and the code that is being compiled.  However LTO is also
> +useful for additional code analysis done by the compiler.  In particular
> +due to interprocedural analysis compiler can produce additional warnings
> +about variables that might be used uninitialized.  Some of these
> +warnings might be "false positives" though and you might need to
> +explicitly initialize variable in order to silence the compiler.

Any word about build speed degradation?

> +
> +Link time optimization can be enabled for whole DPDK framework by
> +setting:
> +
> +.. code-block:: console
> +    CONFIG_ENABLE_LTO=y
> +
> +in config file for the case of make based build and by:
> +
> +.. code-block:: console
> +    meson build -Db_lto=true
> +    ninja -C build

No need to add the ninja step here.

> +for the case of meson based build.

Better to describe the case (make or meson) before the code-block.

> +Please note that turning LTO on causes considerable extension of
> +compilation time.

"compilation time" is not accurate.
When referring to compilation + linking, it is better to use the words
"build time".


> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -56,6 +56,14 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
>  
> +**Added build support for Link Time Optimization.**
> +
> + LTO is an optimization technique used by the compiler to perform whole
> + program analysis and optimization at link time.  In order to do that
> + compilers store their internal representation of the source code that
> + the linker uses at the final stage of compilation process.
> +
> + See :doc:`../prog_guide/lto` for more information:

Please rebase and add a blank line when adding a paragraph.

>  
>  Removed Items
>  -------------

> --- a/lib/librte_distributor/rte_distributor.c
> +++ b/lib/librte_distributor/rte_distributor.c
> -void
> +void __vsym

I replied in v2 about this change. Better to not talk about it here.


I wanted to merge this series in 19.11-rc1 but I think it is better
to move its merge in 19.11-rc2.
Thanks for the work.