From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E3588A0528; Thu, 9 Jul 2020 16:52:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A633B1E8EC; Thu, 9 Jul 2020 16:52:55 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id C1BA11E880 for ; Thu, 9 Jul 2020 16:52:53 +0200 (CEST) IronPort-SDR: gf46O67hWJ0qQOHLs7wmwLBorSxwF4Z3sP45/Xu/raSPWH5TgvPhE5/S5gaqiAZZUeWgB/oyKT LozYoCixjqFg== X-IronPort-AV: E=McAfee;i="6000,8403,9677"; a="135466191" X-IronPort-AV: E=Sophos;i="5.75,331,1589266800"; d="scan'208";a="135466191" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Jul 2020 07:52:52 -0700 IronPort-SDR: oRckbs5NLBbOK8GxnXno3gHpBSDWu7xEEPwOeVrHxlPCULbq5CtLMCADLXVKnEeTsnDciUtgWD a67YGbG2y7FA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,331,1589266800"; d="scan'208";a="324244020" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.213.225.93]) ([10.213.225.93]) by orsmga007.jf.intel.com with ESMTP; 09 Jul 2020 07:52:50 -0700 To: David Marchand Cc: dev , "Ananyev, Konstantin" , Bruce Richardson References: <98b10e12eb46cff65494a94eaf0f04b2dcefd245.1594238610.git.vladimir.medvedkin@intel.com> From: "Medvedkin, Vladimir" Message-ID: <3f68ec75-417b-1587-e13a-07cdeb5056e9@intel.com> Date: Thu, 9 Jul 2020 15:52:50 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4 1/8] eal: introduce zmm type for AVX 512-bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi David, Thanks for review On 09/07/2020 14:48, David Marchand wrote: > On Wed, Jul 8, 2020 at 10:17 PM Vladimir Medvedkin > wrote: >> >> New data type to manipulate 512 bit AVX values. > > The title mentions a "zmm" type that is not added by this patch. > > Maybe instead, "eal/x86: introduce AVX 512-bit type" > Agree > >> >> Signed-off-by: Vladimir Medvedkin >> Acked-by: Konstantin Ananyev >> --- >> lib/librte_eal/x86/include/rte_vect.h | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/lib/librte_eal/x86/include/rte_vect.h b/lib/librte_eal/x86/include/rte_vect.h >> index df5a60762..ae59126bc 100644 >> --- a/lib/librte_eal/x86/include/rte_vect.h >> +++ b/lib/librte_eal/x86/include/rte_vect.h >> @@ -13,6 +13,7 @@ >> >> #include >> #include >> +#include >> #include "generic/rte_vect.h" >> >> #if (defined(__ICC) || \ >> @@ -90,6 +91,26 @@ __extension__ ({ \ >> }) >> #endif /* (defined(__ICC) && __ICC < 1210) */ >> >> +#ifdef __AVX512F__ >> + >> +typedef __m512i __x86_zmm_t; > > We don't need this interim type, using the native __m512 is enough afaics. > Agree > Looking at the whole applied series: > $ git grep -lw __x86_zmm_t > lib/librte_eal/x86/include/rte_vect.h > > >> + >> +#define ZMM_SIZE (sizeof(__x86_zmm_t)) >> +#define ZMM_MASK (ZMM_SIZE - 1) > > Macros in a public header need a RTE_ prefix + this is x86 specific, > then RTE_X86_. > > Looking at the whole applied series: > $ git grep -lw ZMM_SIZE > lib/librte_eal/x86/include/rte_vect.h > $ git grep -lw ZMM_MASK > lib/librte_eal/x86/include/rte_vect.h > > So I wonder if we need to export it or we can instead just #undef > after the struct definition. I think it's better to undef it > > >> + >> +typedef union __rte_x86_zmm { >> + __x86_zmm_t z; >> + ymm_t y[ZMM_SIZE / sizeof(ymm_t)]; >> + xmm_t x[ZMM_SIZE / sizeof(xmm_t)]; >> + uint8_t u8[ZMM_SIZE / sizeof(uint8_t)]; >> + uint16_t u16[ZMM_SIZE / sizeof(uint16_t)]; >> + uint32_t u32[ZMM_SIZE / sizeof(uint32_t)]; >> + uint64_t u64[ZMM_SIZE / sizeof(uint64_t)]; >> + double pd[ZMM_SIZE / sizeof(double)]; >> +} __rte_aligned(ZMM_SIZE) __rte_x86_zmm_t; > > I don't understand this forced alignment statement. > Would not natural alignment be enough, since all fields in this union > have the same size? > Some compilers won't align this union https://mails.dpdk.org/archives/dev/2020-March/159591.html > -- Regards, Vladimir