From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH 1/4] ixgbe: rearrange vector PMD code for x86 Date: Mon, 25 Apr 2016 17:35:56 +0100 Message-ID: <20160425163555.GA11448@bricha3-MOBL3> References: <1461159902-16680-1-git-send-email-jianbo.liu@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, jerin.jacob@caviumnetworks.com, helin.zhang@intel.com, konstantin.ananyev@intel.com To: Jianbo Liu Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 797802B87 for ; Mon, 25 Apr 2016 18:36:31 +0200 (CEST) Content-Disposition: inline In-Reply-To: <1461159902-16680-1-git-send-email-jianbo.liu@linaro.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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. Regards, /Bruce