All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Jianbo Liu <jianbo.liu@linaro.org>
Cc: dev@dpdk.org, jerin.jacob@caviumnetworks.com,
	helin.zhang@intel.com, konstantin.ananyev@intel.com
Subject: Re: [PATCH 1/4] ixgbe: rearrange vector PMD code for x86
Date: Mon, 25 Apr 2016 17:35:56 +0100	[thread overview]
Message-ID: <20160425163555.GA11448@bricha3-MOBL3> (raw)
In-Reply-To: <1461159902-16680-1-git-send-email-jianbo.liu@linaro.org>

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.

Regards,
/Bruce

  parent reply	other threads:[~2016-04-25 16:36 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-20 13:44 [PATCH 1/4] ixgbe: rearrange vector PMD code for x86 Jianbo Liu
2016-04-20 13:45 ` [PATCH 2/4] ixgbe: implement vector PMD for arm architecture Jianbo Liu
2016-04-20 13:45 ` [PATCH 3/4] ixgbe: enable ixgbe vector PMD on ARMv8a platform Jianbo Liu
2016-04-20 13:45 ` [PATCH 4/4] maintainers: claim responsibility for ixgbe vector PMD on ARM Jianbo Liu
2016-04-25 16:35 ` Bruce Richardson [this message]
2016-04-26  8:23   ` [PATCH 1/4] ixgbe: rearrange vector PMD code for x86 Jianbo Liu
2016-04-26 13:50 ` [PATCH v2 " Jianbo Liu
2016-05-03  5:51   ` Jianbo Liu
2016-05-03 16:29   ` Bruce Richardson
2016-04-26 13:55 ` [PATCH v2 2/4] ixgbe: implement vector PMD for arm architecture Jianbo Liu
2016-04-26 13:55 ` [PATCH v2 3/4] ixgbe: enable ixgbe vector PMD on ARMv8a platform Jianbo Liu
2016-04-26 13:55 ` [PATCH v2 4/4] maintainers: claim responsibility for ixgbe vector PMD on ARM Jianbo Liu
2016-05-06  6:25 ` [PATCH v3 0/4] ixgbe: enable " Jianbo Liu
2016-05-06  6:25   ` [PATCH v3 1/4] ixgbe: rearrange vector PMD code for x86 Jianbo Liu
2016-05-06  6:25   ` [PATCH v3 2/4] ixgbe: implement vector PMD for arm architecture Jianbo Liu
2016-05-10 14:49     ` Bruce Richardson
2016-05-11  2:40       ` Jianbo Liu
2016-05-25 12:29     ` Jerin Jacob
2016-05-25 12:53       ` Bruce Richardson
2016-05-26  1:37       ` Jianbo Liu
2016-05-06  6:25   ` [PATCH v3 3/4] ixgbe: enable ixgbe vector PMD on ARMv8a platform Jianbo Liu
2016-05-06  6:25   ` [PATCH v3 4/4] maintainers: claim responsibility for ixgbe vector PMD on ARM Jianbo Liu
2016-05-24 16:10   ` [PATCH v3 0/4] ixgbe: enable " Bruce Richardson
2016-05-24 16:12     ` Bruce Richardson
2016-05-27 10:44       ` Jianbo Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160425163555.GA11448@bricha3-MOBL3 \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=jianbo.liu@linaro.org \
    --cc=konstantin.ananyev@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.