From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2F228A04FD; Mon, 23 May 2022 18:34:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C741F4067B; Mon, 23 May 2022 18:34:36 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 52DCD40156 for ; Mon, 23 May 2022 18:34:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1653323674; x=1684859674; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=X4Hk/+ZB9J7Cw15ig3b1F/pVIlK/hA1o/mMmGL4COek=; b=S7jUyGvcadFBykgIAIzZBrzlPX8dlGtbr/u76MdxDlacp4eT+g2AN51V gfDolzlRZMHf5lFBZ1Uocxbg30lzj1nY3E9PLwRFPFr/1ArdwU0voh01R k0t2UtAFmRDEqzdd36VSsnAIq8hwcLE6KroCmGE1ms2plipbFihjD/c/S PiMkD3nH+61JuxB0Y4YNIAMZW/e69ENd/BhmDmJudoMhIhYJRtesuFVAE oZQSiUcZgC3ZXASjOvV2wrjH7bzNy9gpbHRl9/t+BP87PLUvm3yfRrLqZ 3HRxJpmX9I4CXfN12M6kUzBlwjze7eJc/kaSp86nfu0LJQGxDfLf20i5V Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10356"; a="336336842" X-IronPort-AV: E=Sophos;i="5.91,246,1647327600"; d="scan'208";a="336336842" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2022 09:34:33 -0700 X-IronPort-AV: E=Sophos;i="5.91,246,1647327600"; d="scan'208";a="558735230" Received: from bricha3-mobl.ger.corp.intel.com ([10.55.133.25]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 23 May 2022 09:34:31 -0700 Date: Mon, 23 May 2022 17:34:28 +0100 From: Bruce Richardson To: Timothy McDaniel Cc: jerinj@marvell.com, dev@dpdk.org, Kent Wires Subject: Re: [PATCH v4] event/dlb2: add support for single 512B write of 4 QEs Message-ID: References: <20220409151849.1007602-1-timothy.mcdaniel@intel.com> <20220523160955.3890850-1-timothy.mcdaniel@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220523160955.3890850-1-timothy.mcdaniel@intel.com> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, May 23, 2022 at 11:09:55AM -0500, Timothy McDaniel wrote: > On Xeon, as 512b accesses are available, movdir64 instruction is able to > perform 512b read and write to DLB producer port. In order for movdir64 > to be able to pull its data from store buffers (store-buffer-forwarding) > (before actual write), data should be in single 512b write format. > This commit add change when code is built for Xeon with 512b AVX support > to make single 512b write of all 4 QEs instead of 4x64b writes. > > Signed-off-by: Timothy McDaniel > Acked-by: Kent Wires > === > > Changes since V3: > 1) Renamed dlb2_noavx512.c to dlb2_sve.c, and fixed up meson.build > for new file name. > > Changes since V1: > 1) Split out dlb2_event_build_hcws into two implementations, one > that uses AVX512 instructions, and one that does not. Each implementation > is in its own source file in order to avoid build errors if the compiler > does not support the newer AVX512 instructions. > 2) Update meson.build to and pull in appropriate source file based on > whether the compiler supports AVX512VL > 3) Check if target supports AVX512VL, and use appropriate implementation > based on this runtime check. > --- > drivers/event/dlb2/dlb2.c | 206 +----------------------- > drivers/event/dlb2/dlb2_avx512.c | 267 +++++++++++++++++++++++++++++++ > drivers/event/dlb2/dlb2_priv.h | 8 + > drivers/event/dlb2/dlb2_sve.c | 219 +++++++++++++++++++++++++ > drivers/event/dlb2/meson.build | 14 ++ > 5 files changed, 513 insertions(+), 201 deletions(-) > create mode 100644 drivers/event/dlb2/dlb2_avx512.c > create mode 100644 drivers/event/dlb2/dlb2_sve.c > > diff --git a/drivers/event/dlb2/meson.build b/drivers/event/dlb2/meson.build > index f963589fd3..0ad4d31785 100644 > --- a/drivers/event/dlb2/meson.build > +++ b/drivers/event/dlb2/meson.build > @@ -19,6 +19,20 @@ sources = files( > 'dlb2_selftest.c', > ) > > +dlb2_avx512_support = false > + > +if dpdk_conf.has('RTE_ARCH_X86_64') > + dlb2_avx512_support = ( > + cc.get_define('__AVX512VL__', args: machine_args) != '' > + ) > +endif > + > +if dlb2_avx512_support == true > + sources += files('dlb2_avx512.c') > +else > + sources += files('dlb2_sve.c') > +endif > + > headers = files('rte_pmd_dlb2.h') > > deps += ['mbuf', 'mempool', 'ring', 'pci', 'bus_pci'] I believe this can be improved upon further, since it still does not allow a generic build to opportunistically use the AVX-512 code path. It also makes the runtime check largely pointless as the whole build will have been done with global AVX-512 support, meaning that the binary likely will fail to run if AVX-512 is not available. Instead, I'd recommend doing as other places in DPDK - such as in ACL library, or i40e or ice net drivers - where we not only check the current build support, but also check the compiler support. That way, even if we are building for e.g. a target of AVX2, we can still build the AVX-512 parts using the appropriate compiler flags, and choose them opportunistically at runtime. See the meson.build files in any of the above component directories for examples. Regards, /Bruce