On 6/4/2019 5:19 PM, David Harton (dharton) wrote: > >> -----Original Message----- >> From: Ferruh Yigit <[email protected]> >> Sent: Tuesday, June 04, 2019 12:00 PM >> To: Bruce Richardson <[email protected]>; David Harton (dharton) >> <[email protected]> >> Cc: [email protected]; [email protected]; [email protected] >> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in >> i40e_rxtx.c >> >> On 5/16/2019 3:08 PM, Bruce Richardson wrote: >>> On Wed, May 15, 2019 at 12:13:46PM -0400, David Harton wrote: >>>> Use of weak symbols can hide makefile errors especially when custom >>>> makefiles are used. Removing the use of weak symbols to avoid a stub >>>> function being linked in production code. >>>> >>>> Signed-off-by: David Harton <[email protected]> >>>> --- >>>> >>>> v2 - added CC_AVX2_SUPPORT check to code enabling avx2 vectors >>>> >>> Testing a few compiles here, this breaks when vector mode is disabled, >>> because it's possible that CC_AVX2_SUPPORT=1 when VECTOR=n. I'd >>> suggest adding "ifeq ($(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR),y) ... >>> endif" around the block in the makefile checking for AVX2 support, so >>> that we never set AVX2 unless we have vector support. >> >> Concern is this is pushing vectorization support more to compile time >> configuration. Do we really have to select if to use vector PMD or not in >> compile time? >> >> Can't we get rid of the 'CONFIG_RTE_LIBRTE_I40E_INC_VECTOR' config option >> completely? As done in the ICE driver now. >> >> Isn't it better to compile vectorization support in as much as possible >> and do the vector or scalar path selection in runtime, this patch may >> prevent us to do that, weak functions enables us being more dynamic. > > Choosing a vector dynamically can be done without the use of weak symbols.
No objection then. > > IMHO, weak symbols should really be used for cross library support where a > user wants to override a 3rd party function not within a lib. In fact the > weak symbol usage may preclude supporting all the variants you might > need/want to select. > >> >>> >>> With this change, you can include my ack in v3. >>> >>> /Bruce >>> >>> Acked-by: Bruce Richardson <[email protected]> >>> >
