diff mbox series

[v5,1/8] eal/x86: introduce AVX 512-bit type

Message ID a55cbb5ae404f9453f38e6f2389a82873451506b.1594389240.git.vladimir.medvedkin@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series fib: implement AVX512 vector lookup |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing fail Testing issues
ci/iol-intel-Performance success Performance Testing PASS

Commit Message

Vladimir Medvedkin July 10, 2020, 2:46 p.m. UTC 
  New data type to manipulate 512 bit AVX values.

Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
 lib/librte_eal/x86/include/rte_vect.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Thomas Monjalon July 10, 2020, 9:49 p.m. UTC | #1 
Please Cc those who participated in the review previously.
Adding Ray, Jerin, David.

10/07/2020 16:46, Vladimir Medvedkin:
> New data type to manipulate 512 bit AVX values.
[...]
> +#ifdef __AVX512F__
> +
> +#define	RTE_X86_ZMM_SIZE	(sizeof(__m512i))
> +#define	RTE_X86_ZMM_MASK	(ZMM_SIZE - 1)

Why do you use tabs?

> +
> +typedef union __rte_x86_zmm  {

Double space

> +	__m512i	 z;
> +	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
> +	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
> +	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
> +	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
> +	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
> +	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
> +	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
> +} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
> +
> +#endif /* __AVX512F__ */

You were supposed to undef the macros above.

Vladimir, after your recent contributions,
it seems you are not interested in details.
Please understand we have to maintain a project with consistency
and good doc. Please pay attention, thanks.
Vladimir Medvedkin July 13, 2020, 10:23 a.m. UTC | #2 
Hi Thomas,

On 10/07/2020 22:49, Thomas Monjalon wrote:
> Please Cc those who participated in the review previously.
> Adding Ray, Jerin, David.
> 
> 10/07/2020 16:46, Vladimir Medvedkin:
>> New data type to manipulate 512 bit AVX values.
> [...]
>> +#ifdef __AVX512F__
>> +
>> +#define	RTE_X86_ZMM_SIZE	(sizeof(__m512i))
>> +#define	RTE_X86_ZMM_MASK	(ZMM_SIZE - 1)
> 
> Why do you use tabs?

Will resend v6

> 
>> +
>> +typedef union __rte_x86_zmm  {
> 
> Double space

Will fix in v6

> 
>> +	__m512i	 z;
>> +	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
>> +	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
>> +	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
>> +	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
>> +	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
>> +	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
>> +	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
>> +} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
>> +
>> +#endif /* __AVX512F__ */
> 
> You were supposed to undef the macros above.

It was intentional. It could be used later by other libs, like XMM_SIZE:
git grep -lw XMM_SIZE
lib/librte_acl/acl_gen.c
lib/librte_acl/acl_run.h
lib/librte_acl/rte_acl.h
lib/librte_eal/arm/include/rte_vect.h
lib/librte_eal/ppc/include/rte_vect.h
lib/librte_eal/x86/include/rte_vect.h
lib/librte_hash/rte_thash.h

> 
> Vladimir, after your recent contributions,
> it seems you are not interested in details.
> Please understand we have to maintain a project with consistency
> and good doc. Please pay attention, thanks.
> 
>
Thomas Monjalon July 13, 2020, 10:25 a.m. UTC | #3 
13/07/2020 12:23, Medvedkin, Vladimir:
> Hi Thomas,
> 
> On 10/07/2020 22:49, Thomas Monjalon wrote:
> > Please Cc those who participated in the review previously.
> > Adding Ray, Jerin, David.
> > 
> > 10/07/2020 16:46, Vladimir Medvedkin:
> >> +	__m512i	 z;
> >> +	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
> >> +	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
> >> +	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
> >> +	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
> >> +	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
> >> +	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
> >> +	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
> >> +} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
> >> +
> >> +#endif /* __AVX512F__ */
> > 
> > You were supposed to undef the macros above.
> 
> It was intentional. It could be used later by other libs, like XMM_SIZE:
> git grep -lw XMM_SIZE
> lib/librte_acl/acl_gen.c
> lib/librte_acl/acl_run.h
> lib/librte_acl/rte_acl.h
> lib/librte_eal/arm/include/rte_vect.h
> lib/librte_eal/ppc/include/rte_vect.h
> lib/librte_eal/x86/include/rte_vect.h
> lib/librte_hash/rte_thash.h

OK. Was it agreed with David to NOT undef?
I may have missed this part.
Vladimir Medvedkin July 13, 2020, 10:39 a.m. UTC | #4 
On 13/07/2020 11:25, Thomas Monjalon wrote:
> 13/07/2020 12:23, Medvedkin, Vladimir:
>> Hi Thomas,
>>
>> On 10/07/2020 22:49, Thomas Monjalon wrote:
>>> Please Cc those who participated in the review previously.
>>> Adding Ray, Jerin, David.
>>>
>>> 10/07/2020 16:46, Vladimir Medvedkin:
>>>> +	__m512i	 z;
>>>> +	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
>>>> +	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
>>>> +	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
>>>> +	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
>>>> +	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
>>>> +	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
>>>> +	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
>>>> +} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
>>>> +
>>>> +#endif /* __AVX512F__ */
>>>
>>> You were supposed to undef the macros above.
>>
>> It was intentional. It could be used later by other libs, like XMM_SIZE:
>> git grep -lw XMM_SIZE
>> lib/librte_acl/acl_gen.c
>> lib/librte_acl/acl_run.h
>> lib/librte_acl/rte_acl.h
>> lib/librte_eal/arm/include/rte_vect.h
>> lib/librte_eal/ppc/include/rte_vect.h
>> lib/librte_eal/x86/include/rte_vect.h
>> lib/librte_hash/rte_thash.h
> 
> OK. Was it agreed with David to NOT undef?
> I may have missed this part.
> 

As I can understand David had no objections to export it. I think it 
could be useful for some libs to have those macros. Please correct me if 
I'm wrong.

>
Ananyev, Konstantin July 13, 2020, 10:45 a.m. UTC | #5 
> 
> On 13/07/2020 11:25, Thomas Monjalon wrote:
> > 13/07/2020 12:23, Medvedkin, Vladimir:
> >> Hi Thomas,
> >>
> >> On 10/07/2020 22:49, Thomas Monjalon wrote:
> >>> Please Cc those who participated in the review previously.
> >>> Adding Ray, Jerin, David.
> >>>
> >>> 10/07/2020 16:46, Vladimir Medvedkin:
> >>>> +	__m512i	 z;
> >>>> +	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
> >>>> +	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
> >>>> +	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
> >>>> +	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
> >>>> +	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
> >>>> +	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
> >>>> +	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
> >>>> +} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
> >>>> +
> >>>> +#endif /* __AVX512F__ */
> >>>
> >>> You were supposed to undef the macros above.
> >>
> >> It was intentional. It could be used later by other libs, like XMM_SIZE:
> >> git grep -lw XMM_SIZE
> >> lib/librte_acl/acl_gen.c
> >> lib/librte_acl/acl_run.h
> >> lib/librte_acl/rte_acl.h
> >> lib/librte_eal/arm/include/rte_vect.h
> >> lib/librte_eal/ppc/include/rte_vect.h
> >> lib/librte_eal/x86/include/rte_vect.h
> >> lib/librte_hash/rte_thash.h
> >
> > OK. Was it agreed with David to NOT undef?
> > I may have missed this part.
> >
> 
> As I can understand David had no objections to export it. I think it
> could be useful for some libs to have those macros. 

+1
diff mbox series

Patch

diff --git a/lib/librte_eal/x86/include/rte_vect.h b/lib/librte_eal/x86/include/rte_vect.h
index df5a60762..1b2af7138 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,24 @@  __extension__ ({                 \
 })
 #endif /* (defined(__ICC) && __ICC < 1210) */
 
+#ifdef __AVX512F__
+
+#define	RTE_X86_ZMM_SIZE	(sizeof(__m512i))
+#define	RTE_X86_ZMM_MASK	(ZMM_SIZE - 1)
+
+typedef union __rte_x86_zmm  {
+	__m512i	 z;
+	ymm_t    y[RTE_X86_ZMM_SIZE / sizeof(ymm_t)];
+	xmm_t    x[RTE_X86_ZMM_SIZE / sizeof(xmm_t)];
+	uint8_t  u8[RTE_X86_ZMM_SIZE / sizeof(uint8_t)];
+	uint16_t u16[RTE_X86_ZMM_SIZE / sizeof(uint16_t)];
+	uint32_t u32[RTE_X86_ZMM_SIZE / sizeof(uint32_t)];
+	uint64_t u64[RTE_X86_ZMM_SIZE / sizeof(uint64_t)];
+	double   pd[RTE_X86_ZMM_SIZE / sizeof(double)];
+} __rte_aligned(RTE_X86_ZMM_SIZE) __rte_x86_zmm_t;
+
+#endif /* __AVX512F__ */
+
 #ifdef __cplusplus
 }
 #endif