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

[Andrzej Ostruszka]

Thank you Thomas for the comments.  Please see below for replies.

On 10/27/19 12:47 PM, Thomas Monjalon wrote:
> 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.

Will add it (between "writing efficient code" and "profile app").

>> @@ -0,0 +1,36 @@
> 
> You are missing the mandatory SPDX and Copyright header.

Will add.

>> +Link Time Optimization
>> +======================
>> +
>> +The DPDK framework supports compilation with link time optimization
> 
> Instead of "The DPDK framework", I prefer just "DPDK".

OK

>> +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

That was not a reference to some formally defined "capabilities" just an
"obvious" statement that in order to benefit from LTO one needs a
compiler "capable of" performing it.  I will rephrase this paragraph as:

-8<-------------
The DPDK supports compilation with link time optimization turned on.
This depends obviously on the ability of the compiler to do "whole
program" optimization at link time and is available only for compilers
that support that feature (gcc and icc).
To be more specific, compiler (in addition to performing LTO) have to
support creation of ELF objects containing 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).
-8<-------------

Will it suffice?  If not, then please suggest wording to be used.

>> +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.

OK

>> +specific compiler have to support creation of ELF objects containing
> 
> A comma is missing after "specific".

OK

>> +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?

It is mentioned at the end of this file - I will move it here.

>> +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.

OK

>> +for the case of meson based build.
> 
> Better to describe the case (make or meson) before the code-block.

OK

>> +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".

OK

>> --- 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.

OK

[...]
> I wanted to merge this series in 19.11-rc1 but I think it is better
> to move its merge in 19.11-rc2.

OK.  I'll send the next version today.

Thank you again for the comments.

Regards
Andrzej