Re: [dpdk-dev] [PATCH 1/4] ixgbe: rearrange vector PMD code for x86

[Jianbo Liu <jianbo.liu@linaro.org>]

On 26 April 2016 at 00:35, Bruce Richardson <bruce.richardson@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@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