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

[Andrzej Ostruszka]

On 10/27/19 12:31 PM, Thomas Monjalon wrote:
> 18/09/2019 15:32, Ray Kinsella:
>> this is cool, good work.
>> comments below.
>>
>> On 17/09/2019 08:57, Andrzej Ostruszka wrote:
>>> --- a/lib/librte_distributor/rte_distributor.c
>>> +++ b/lib/librte_distributor/rte_distributor.c
>>> @@ -32,7 +32,7 @@ EAL_REGISTER_TAILQ(rte_dist_burst_tailq)
>>>  
>>>  /**** Burst Packet APIs called by workers ****/
>>>  
>>> -void
>>> +void __vsym
>>
>> all these additional __vsym annotations looks like they belong in a
>> seperate patch, as they are fixing a bug and are not directly related to
>> adding LTO the build system.
> 
> Andrzej, you did not reply to this question.
> This is a real blocker for merging this series.

Thomas, thank you for the reminder.  Somehow that comment has escaped me
- although I've read it then.

> Should __vsym addition be in a separate patch?

I'm fine both ways.  You could argue that:
- it is a bug since '__vsym' clearly annotates the function as being
  used as a particular version of a symbol and as such it was missing
- or as a part of enablement for LTO since without it compiler/linker
  should not be removing given function and '__vsym' really is just a
  "attribute(used)" to tell optimizing compiler/linker that this
  function should not be removed.

Since you raised that question I'm guessing that you prefer it to be in
a separate patch - so, unless you object now, I'm going to split it in
the next version and have it as a first patch.

> Should we document its use in rte_function_versioning.h
> and versioning.rst?

Yes, I think so.  I'll add that.

Again, thank you for the reminder and comments.

Regards
Andrzej