DPDK patches and discussions
 help / color / mirror / Atom feed
* [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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git