> -----Original Message----- > From: Ferruh Yigit <ferruh.yi...@intel.com> > Sent: Tuesday, June 04, 2019 12:00 PM > To: Bruce Richardson <bruce.richard...@intel.com>; David Harton (dharton) > <dhar...@cisco.com> > Cc: dev@dpdk.org; beilei.x...@intel.com; qi.z.zh...@intel.com > 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 <dhar...@cisco.com> > >> --- > >> > >> 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. 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 <bruce.richard...@intel.com> > >
