Re: [dpdk-dev] [PATCH v4 1/8] eal: introduce zmm type for AVX 512-bit
From: David Marchand <hidden>
Date: 2020-07-09 13:48:39
On Wed, Jul 8, 2020 at 10:17 PM Vladimir Medvedkin [off-list ref] 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"
quoted hunk ↗ jump to hunk
Signed-off-by: Vladimir Medvedkin <redacted> Acked-by: Konstantin Ananyev <redacted> --- 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 <stdint.h> #include <rte_config.h> +#include <rte_common.h> #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. 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.
+
+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? -- David Marchand