* [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
@ 2021-01-12 7:24 kumar amber
2021-03-24 21:20 ` Thomas Monjalon
2021-03-24 22:59 ` Wang, Yipeng1
0 siblings, 2 replies; 7+ messages in thread
From: kumar amber @ 2021-01-12 7:24 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson
add _SSE42_ flag to enable compilation of
sse42 specific instructions only on supported
architecture
Signed-off-by: kumar amber <kumar.amber@intel.com>
---
lib/librte_hash/rte_hash_crc.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
index 3e131aa6bb..e9f063780c 100644
--- a/lib/librte_hash/rte_hash_crc.h
+++ b/lib/librte_hash/rte_hash_crc.h
@@ -358,7 +358,7 @@ crc32c_2words(uint64_t data, uint32_t init_val)
return crc;
}
-#if defined(RTE_ARCH_X86)
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
static inline uint32_t
crc32c_sse42_u8(uint8_t data, uint32_t init_val)
{
@@ -404,7 +404,7 @@ crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val)
}
#endif
-#ifdef RTE_ARCH_X86_64
+#if defined(RTE_ARCH_X86_64) && defined(__SSE42__)
static inline uint32_t
crc32c_sse42_u64(uint64_t data, uint64_t init_val)
{
@@ -442,7 +442,7 @@ static uint8_t crc32_alg = CRC32_SW;
static inline void
rte_hash_crc_set_alg(uint8_t alg)
{
-#if defined(RTE_ARCH_X86)
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (alg == CRC32_SSE42_x64 &&
!rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T))
alg = CRC32_SSE42;
@@ -471,7 +471,7 @@ RTE_INIT(rte_hash_crc_init_alg)
static inline uint32_t
rte_hash_crc_1byte(uint8_t data, uint32_t init_val)
{
-#if defined RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (likely(crc32_alg & CRC32_SSE42))
return crc32c_sse42_u8(data, init_val);
#endif
@@ -494,7 +494,7 @@ rte_hash_crc_1byte(uint8_t data, uint32_t init_val)
static inline uint32_t
rte_hash_crc_2byte(uint16_t data, uint32_t init_val)
{
-#if defined RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (likely(crc32_alg & CRC32_SSE42))
return crc32c_sse42_u16(data, init_val);
#endif
@@ -517,7 +517,7 @@ rte_hash_crc_2byte(uint16_t data, uint32_t init_val)
static inline uint32_t
rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
{
-#if defined RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (likely(crc32_alg & CRC32_SSE42))
return crc32c_sse42_u32(data, init_val);
#endif
@@ -540,12 +540,12 @@ rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
static inline uint32_t
rte_hash_crc_8byte(uint64_t data, uint32_t init_val)
{
-#ifdef RTE_ARCH_X86_64
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (likely(crc32_alg == CRC32_SSE42_x64))
return crc32c_sse42_u64(data, init_val);
#endif
-#if defined RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(__SSE42__)
if (likely(crc32_alg & CRC32_SSE42))
return crc32c_sse42_u64_mimic(data, init_val);
#endif
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-01-12 7:24 [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture kumar amber
@ 2021-03-24 21:20 ` Thomas Monjalon
2021-03-24 22:59 ` Wang, Yipeng1
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2021-03-24 21:20 UTC (permalink / raw)
To: kumar amber; +Cc: dev, bruce.richardson, Yipeng Wang, Sameh Gobriel
There was no review of this patch in last 2 months.
+Cc Yipeng and Sameh
12/01/2021 08:24, kumar amber:
> add _SSE42_ flag to enable compilation of
> sse42 specific instructions only on supported
> architecture
>
> Signed-off-by: kumar amber <kumar.amber@intel.com>
> ---
> lib/librte_hash/rte_hash_crc.h | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
> index 3e131aa6bb..e9f063780c 100644
> --- a/lib/librte_hash/rte_hash_crc.h
> +++ b/lib/librte_hash/rte_hash_crc.h
> @@ -358,7 +358,7 @@ crc32c_2words(uint64_t data, uint32_t init_val)
> return crc;
> }
>
> -#if defined(RTE_ARCH_X86)
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> static inline uint32_t
> crc32c_sse42_u8(uint8_t data, uint32_t init_val)
> {
> @@ -404,7 +404,7 @@ crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val)
> }
> #endif
>
> -#ifdef RTE_ARCH_X86_64
> +#if defined(RTE_ARCH_X86_64) && defined(__SSE42__)
> static inline uint32_t
> crc32c_sse42_u64(uint64_t data, uint64_t init_val)
> {
> @@ -442,7 +442,7 @@ static uint8_t crc32_alg = CRC32_SW;
> static inline void
> rte_hash_crc_set_alg(uint8_t alg)
> {
> -#if defined(RTE_ARCH_X86)
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (alg == CRC32_SSE42_x64 &&
> !rte_cpu_get_flag_enabled(RTE_CPUFLAG_EM64T))
> alg = CRC32_SSE42;
> @@ -471,7 +471,7 @@ RTE_INIT(rte_hash_crc_init_alg)
> static inline uint32_t
> rte_hash_crc_1byte(uint8_t data, uint32_t init_val)
> {
> -#if defined RTE_ARCH_X86
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg & CRC32_SSE42))
> return crc32c_sse42_u8(data, init_val);
> #endif
> @@ -494,7 +494,7 @@ rte_hash_crc_1byte(uint8_t data, uint32_t init_val)
> static inline uint32_t
> rte_hash_crc_2byte(uint16_t data, uint32_t init_val)
> {
> -#if defined RTE_ARCH_X86
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg & CRC32_SSE42))
> return crc32c_sse42_u16(data, init_val);
> #endif
> @@ -517,7 +517,7 @@ rte_hash_crc_2byte(uint16_t data, uint32_t init_val)
> static inline uint32_t
> rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
> {
> -#if defined RTE_ARCH_X86
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg & CRC32_SSE42))
> return crc32c_sse42_u32(data, init_val);
> #endif
> @@ -540,12 +540,12 @@ rte_hash_crc_4byte(uint32_t data, uint32_t init_val)
> static inline uint32_t
> rte_hash_crc_8byte(uint64_t data, uint32_t init_val)
> {
> -#ifdef RTE_ARCH_X86_64
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg == CRC32_SSE42_x64))
> return crc32c_sse42_u64(data, init_val);
> #endif
>
> -#if defined RTE_ARCH_X86
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg & CRC32_SSE42))
> return crc32c_sse42_u64_mimic(data, init_val);
> #endif
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-01-12 7:24 [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture kumar amber
2021-03-24 21:20 ` Thomas Monjalon
@ 2021-03-24 22:59 ` Wang, Yipeng1
2021-03-25 8:06 ` Thomas Monjalon
1 sibling, 1 reply; 7+ messages in thread
From: Wang, Yipeng1 @ 2021-03-24 22:59 UTC (permalink / raw)
To: Amber, Kumar, dev; +Cc: Richardson, Bruce, Thomas Monjalon, Gobriel, Sameh
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of kumar amber
> Sent: Monday, January 11, 2021 11:25 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
>
> add _SSE42_ flag to enable compilation of
> sse42 specific instructions only on supported architecture
>
> Signed-off-by: kumar amber <kumar.amber@intel.com>
> ---
> lib/librte_hash/rte_hash_crc.h | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
> index 3e131aa6bb..e9f063780c 100644
> --- a/lib/librte_hash/rte_hash_crc.h
> +++ b/lib/librte_hash/rte_hash_crc.h
> @@ -358,7 +358,7 @@ crc32c_2words(uint64_t data, uint32_t init_val)
> return crc;
> }
>
> -#if defined(RTE_ARCH_X86)
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> static inline uint32_t
> crc32c_sse42_u8(uint8_t data, uint32_t init_val) { @@ -404,7 +404,7 @@
> crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val) } #endif
...
> -#if defined RTE_ARCH_X86
> +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> if (likely(crc32_alg & CRC32_SSE42))
> return crc32c_sse42_u64_mimic(data, init_val); #endif
> --
> 2.25.1
[Wang, Yipeng]
Hi, Kumar, thanks for the patch.
I think the minimum required machine for x86 is sse4.2 compatible already. So I wonder if we really need this.
Also, I think the right way to check machine flag in DPDK should be:
#If defined (RTE_MACHINE_CPUFLAG_SSE4_2)
Instead of using compiler dependent macro.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-03-24 22:59 ` Wang, Yipeng1
@ 2021-03-25 8:06 ` Thomas Monjalon
2021-04-08 22:41 ` Thomas Monjalon
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2021-03-25 8:06 UTC (permalink / raw)
To: Amber, Kumar, Wang, Yipeng1; +Cc: dev, Richardson, Bruce, Gobriel, Sameh
24/03/2021 23:59, Wang, Yipeng1:
> From: kumar amber
> >
> > add _SSE42_ flag to enable compilation of
> > sse42 specific instructions only on supported architecture
> >
> > Signed-off-by: kumar amber <kumar.amber@intel.com>
> > ---
> > lib/librte_hash/rte_hash_crc.h | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_hash/rte_hash_crc.h b/lib/librte_hash/rte_hash_crc.h
> > index 3e131aa6bb..e9f063780c 100644
> > --- a/lib/librte_hash/rte_hash_crc.h
> > +++ b/lib/librte_hash/rte_hash_crc.h
> > @@ -358,7 +358,7 @@ crc32c_2words(uint64_t data, uint32_t init_val)
> > return crc;
> > }
> >
> > -#if defined(RTE_ARCH_X86)
> > +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> > static inline uint32_t
> > crc32c_sse42_u8(uint8_t data, uint32_t init_val) { @@ -404,7 +404,7 @@
> > crc32c_sse42_u64_mimic(uint64_t data, uint64_t init_val) } #endif
>
> ...
>
> > -#if defined RTE_ARCH_X86
> > +#if defined(RTE_ARCH_X86) && defined(__SSE42__)
> > if (likely(crc32_alg & CRC32_SSE42))
> > return crc32c_sse42_u64_mimic(data, init_val); #endif
> > --
> > 2.25.1
>
> [Wang, Yipeng]
> Hi, Kumar, thanks for the patch.
> I think the minimum required machine for x86 is sse4.2 compatible already. So I wonder if we really need this.
Yes, that's why I don't understand this patch.
> Also, I think the right way to check machine flag in DPDK should be:
> #If defined (RTE_MACHINE_CPUFLAG_SSE4_2)
These macros have been removed in DPDK 20.11.
> Instead of using compiler dependent macro.
Compiler macros are well standardized, it is OK.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-03-25 8:06 ` Thomas Monjalon
@ 2021-04-08 22:41 ` Thomas Monjalon
2021-04-08 23:01 ` Stephen Hemminger
2021-04-09 3:17 ` Amber, Kumar
0 siblings, 2 replies; 7+ messages in thread
From: Thomas Monjalon @ 2021-04-08 22:41 UTC (permalink / raw)
To: Amber, Kumar; +Cc: Wang, Yipeng1, dev, Richardson, Bruce, Gobriel, Sameh
25/03/2021 09:06, Thomas Monjalon:
> 24/03/2021 23:59, Wang, Yipeng1:
> > From: kumar amber
> > >
> > > add _SSE42_ flag to enable compilation of
> > > sse42 specific instructions only on supported architecture
> > >
> > > Signed-off-by: kumar amber <kumar.amber@intel.com>
[...]
> > [Wang, Yipeng]
> > Hi, Kumar, thanks for the patch.
> > I think the minimum required machine for x86 is sse4.2 compatible already. So I wonder if we really need this.
>
> Yes, that's why I don't understand this patch.
Any comment? Should we reject the patch?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-04-08 22:41 ` Thomas Monjalon
@ 2021-04-08 23:01 ` Stephen Hemminger
2021-04-09 3:17 ` Amber, Kumar
1 sibling, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2021-04-08 23:01 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Amber, Kumar, Wang, Yipeng1, dev, Richardson, Bruce, Gobriel, Sameh
On Fri, 09 Apr 2021 00:41:34 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> 25/03/2021 09:06, Thomas Monjalon:
> > 24/03/2021 23:59, Wang, Yipeng1:
> > > From: kumar amber
> > > >
> > > > add _SSE42_ flag to enable compilation of
> > > > sse42 specific instructions only on supported architecture
> > > >
> > > > Signed-off-by: kumar amber <kumar.amber@intel.com>
> [...]
> > > [Wang, Yipeng]
> > > Hi, Kumar, thanks for the patch.
> > > I think the minimum required machine for x86 is sse4.2 compatible already. So I wonder if we really need this.
> >
> > Yes, that's why I don't understand this patch.
>
> Any comment? Should we reject the patch?
>
>
>
The only use case would be running DPDK in a VM for a hypervisor that
disables those instructions in guest. Some old crufty hypervisors don't
handle it??
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
2021-04-08 22:41 ` Thomas Monjalon
2021-04-08 23:01 ` Stephen Hemminger
@ 2021-04-09 3:17 ` Amber, Kumar
1 sibling, 0 replies; 7+ messages in thread
From: Amber, Kumar @ 2021-04-09 3:17 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Wang, Yipeng1, dev, Richardson, Bruce, Gobriel, Sameh
Hi Thomas ,
Pls reject it
-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Friday, April 9, 2021 4:12 AM
To: Amber, Kumar <kumar.amber@intel.com>
Cc: Wang, Yipeng1 <yipeng1.wang@intel.com>; dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>
Subject: Re: [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture
25/03/2021 09:06, Thomas Monjalon:
> 24/03/2021 23:59, Wang, Yipeng1:
> > From: kumar amber
> > >
> > > add _SSE42_ flag to enable compilation of
> > > sse42 specific instructions only on supported architecture
> > >
> > > Signed-off-by: kumar amber <kumar.amber@intel.com>
[...]
> > [Wang, Yipeng]
> > Hi, Kumar, thanks for the patch.
> > I think the minimum required machine for x86 is sse4.2 compatible already. So I wonder if we really need this.
>
> Yes, that's why I don't understand this patch.
Any comment? Should we reject the patch?
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-09 3:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 7:24 [dpdk-dev] [PATCH v1] lib/hash: support non sse42 cpu architecture kumar amber
2021-03-24 21:20 ` Thomas Monjalon
2021-03-24 22:59 ` Wang, Yipeng1
2021-03-25 8:06 ` Thomas Monjalon
2021-04-08 22:41 ` Thomas Monjalon
2021-04-08 23:01 ` Stephen Hemminger
2021-04-09 3:17 ` Amber, Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).