28/10/2019 09:36, 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.
Thank you, a separate patch with doc update is very welcome.
