On 26 April 2016 at 00:35, Bruce Richardson <bruce.richardson at intel.com> wrote: > On Wed, Apr 20, 2016 at 09:44:59PM +0800, Jianbo Liu wrote: >> move SSE-dependent code to new file "ixgbe_rxtx_vec_sse.h" >> >> Signed-off-by: Jianbo Liu <jianbo.liu at linaro.org> >> --- >> drivers/net/ixgbe/ixgbe_rxtx_vec.c | 369 +---------------------------- >> drivers/net/ixgbe/ixgbe_rxtx_vec_sse.h | 408 >> +++++++++++++++++++++++++++++++++ >> 2 files changed, 409 insertions(+), 368 deletions(-) >> create mode 100644 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.h >> > Hi Jianbo, > > functionally I've given this a quick sanity test and see no issues with > performance > on the x86(_64) side of things. > > However, in terms of how the driver split in done in this set of patches, I > think > it might be better to reverse what goes in the header files and in the .c > files. > Rather than having the common code in the .c file and the arch specific code > in > the header file, I think the common code should be in a header file and the > arch specific code in a .c file. > > The reason for this is the need for possibly different compiler flags to be > passed for the vector drivers from the makefile e.g. as is done by my patchset > for i40e [http://dpdk.org/dev/patchwork/patch/12082/]. This would be a bit > more > awkward if that one C file is shared by multiple architectures, as we'd have > architecture specific branches in both makefile and C file. As well as that, > the possibility exists of multiple vector drivers for one architecture, e.g. > an SSE and AVX driver for x86_64 with selection of code patch at runtime as > done > by the ACL library. In that case, you want multiple vector code paths compiled > with different CFLAG overrides, which necessitates different C files. > > Therefore, I think using a C file per instruction set/architecture, rather > than > a header file per arch may be more expandable in future. >
Good suggestion. I will submit v2 later. Thanks! Jianbo
