DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
@ 2015-03-02  9:03 zhihong.wang
  2015-03-02 10:32 ` Bruce Richardson
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: zhihong.wang @ 2015-03-02  9:03 UTC (permalink / raw)
  To: dev

Fixed strict-aliasing rules breaking errors for some GCC version.

Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
 .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index 69a5c6f..f412099 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
 static inline void *
 rte_memcpy(void *dst, const void *src, size_t n)
 {
+	uintptr_t dstu = (uintptr_t)dst;
+	uintptr_t srcu = (uintptr_t)src;
 	void *ret = dst;
 	int dstofss;
 	int bits;
@@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
 	 */
 	if (n < 16) {
 		if (n & 0x01) {
-			*(uint8_t *)dst = *(const uint8_t *)src;
-			src = (const uint8_t *)src + 1;
-			dst = (uint8_t *)dst + 1;
+			*(uint8_t *)dstu = *(const uint8_t *)srcu;
+			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
+			dstu = (uintptr_t)((uint8_t *)dstu + 1);
 		}
 		if (n & 0x02) {
-			*(uint16_t *)dst = *(const uint16_t *)src;
-			src = (const uint16_t *)src + 1;
-			dst = (uint16_t *)dst + 1;
+			*(uint16_t *)dstu = *(const uint16_t *)srcu;
+			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
+			dstu = (uintptr_t)((uint16_t *)dstu + 1);
 		}
 		if (n & 0x04) {
-			*(uint32_t *)dst = *(const uint32_t *)src;
-			src = (const uint32_t *)src + 1;
-			dst = (uint32_t *)dst + 1;
+			*(uint32_t *)dstu = *(const uint32_t *)srcu;
+			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
+			dstu = (uintptr_t)((uint32_t *)dstu + 1);
 		}
 		if (n & 0x08) {
-			*(uint64_t *)dst = *(const uint64_t *)src;
+			*(uint64_t *)dstu = *(const uint64_t *)srcu;
 		}
 		return ret;
 	}
@@ -458,6 +460,8 @@ static inline void *
 rte_memcpy(void *dst, const void *src, size_t n)
 {
 	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
+	uintptr_t dstu = (uintptr_t)dst;
+	uintptr_t srcu = (uintptr_t)src;
 	void *ret = dst;
 	int dstofss;
 	int srcofs;
@@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
 	 */
 	if (n < 16) {
 		if (n & 0x01) {
-			*(uint8_t *)dst = *(const uint8_t *)src;
-			src = (const uint8_t *)src + 1;
-			dst = (uint8_t *)dst + 1;
+			*(uint8_t *)dstu = *(const uint8_t *)srcu;
+			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
+			dstu = (uintptr_t)((uint8_t *)dstu + 1);
 		}
 		if (n & 0x02) {
-			*(uint16_t *)dst = *(const uint16_t *)src;
-			src = (const uint16_t *)src + 1;
-			dst = (uint16_t *)dst + 1;
+			*(uint16_t *)dstu = *(const uint16_t *)srcu;
+			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
+			dstu = (uintptr_t)((uint16_t *)dstu + 1);
 		}
 		if (n & 0x04) {
-			*(uint32_t *)dst = *(const uint32_t *)src;
-			src = (const uint32_t *)src + 1;
-			dst = (uint32_t *)dst + 1;
+			*(uint32_t *)dstu = *(const uint32_t *)srcu;
+			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
+			dstu = (uintptr_t)((uint32_t *)dstu + 1);
 		}
 		if (n & 0x08) {
-			*(uint64_t *)dst = *(const uint64_t *)src;
+			*(uint64_t *)dstu = *(const uint64_t *)srcu;
 		}
 		return ret;
 	}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02  9:03 [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking zhihong.wang
@ 2015-03-02 10:32 ` Bruce Richardson
  2015-03-02 12:32   ` Pawel Wodkowski
  2015-03-04  2:07   ` Wang, Zhihong
  2015-03-05 17:33 ` Thomas Monjalon
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Bruce Richardson @ 2015-03-02 10:32 UTC (permalink / raw)
  To: zhihong.wang; +Cc: dev

On Mon, Mar 02, 2015 at 05:03:50PM +0800, zhihong.wang@intel.com wrote:
> Fixed strict-aliasing rules breaking errors for some GCC version.
> 

This looks messy. Also, I believe the definition of memcpy should include
the "restrict" keyword to indicate that source and dest can't overlap. Might
that help fix the issue?

/Bruce

> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
>  .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> index 69a5c6f..f412099 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
>  static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>  	void *ret = dst;
>  	int dstofss;
>  	int bits;
> @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>  	 */
>  	if (n < 16) {
>  		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>  		}
>  		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>  		}
>  		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>  		}
>  		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>  		}
>  		return ret;
>  	}
> @@ -458,6 +460,8 @@ static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
>  	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>  	void *ret = dst;
>  	int dstofss;
>  	int srcofs;
> @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>  	 */
>  	if (n < 16) {
>  		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>  		}
>  		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>  		}
>  		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>  		}
>  		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>  		}
>  		return ret;
>  	}
> -- 
> 1.9.3
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02 10:32 ` Bruce Richardson
@ 2015-03-02 12:32   ` Pawel Wodkowski
  2015-03-04  5:57     ` Wang, Zhihong
  2015-03-04  2:07   ` Wang, Zhihong
  1 sibling, 1 reply; 10+ messages in thread
From: Pawel Wodkowski @ 2015-03-02 12:32 UTC (permalink / raw)
  To: Bruce Richardson, zhihong.wang; +Cc: dev

On 2015-03-02 11:32, Bruce Richardson wrote:
> On Mon, Mar 02, 2015 at 05:03:50PM +0800, zhihong.wang@intel.com wrote:
>> Fixed strict-aliasing rules breaking errors for some GCC version.
>>
>
> This looks messy. Also, I believe the definition of memcpy should include
> the "restrict" keyword to indicate that source and dest can't overlap. Might
> that help fix the issue?
>

Is this error related with overlapping or casting 'void *' to 'uintXX_t 
*' that make compiler report aliasing rule breaking?

>
>> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
>> ---
>>   .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
>>   1 file changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
>> index 69a5c6f..f412099 100644
>> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
>> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
>> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
>>   static inline void *
>>   rte_memcpy(void *dst, const void *src, size_t n)
>>   {
>> +	uintptr_t dstu = (uintptr_t)dst;
>> +	uintptr_t srcu = (uintptr_t)src;

If so maybe using union here would be good solution or 'char *'.

-- 
Pawel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02 10:32 ` Bruce Richardson
  2015-03-02 12:32   ` Pawel Wodkowski
@ 2015-03-04  2:07   ` Wang, Zhihong
  2015-03-04 10:18     ` Bruce Richardson
  1 sibling, 1 reply; 10+ messages in thread
From: Wang, Zhihong @ 2015-03-04  2:07 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev



> -----Original Message-----
> From: Richardson, Bruce
> Sent: Monday, March 02, 2015 6:32 PM
> To: Wang, Zhihong
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules
> breaking
> 
> On Mon, Mar 02, 2015 at 05:03:50PM +0800, zhihong.wang@intel.com wrote:
> > Fixed strict-aliasing rules breaking errors for some GCC version.
> >
> 
> This looks messy. Also, I believe the definition of memcpy should include the
> "restrict" keyword to indicate that source and dest can't overlap. Might that
> help fix the issue?

It's actually caused by casting void * to multiple other pointer types.

> 
> /Bruce
> 
> > Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> > ---
> >  .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----
> ------
> >  1 file changed, 24 insertions(+), 20 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > index 69a5c6f..f412099 100644
> > --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src,
> > size_t n)  static inline void *  rte_memcpy(void *dst, const void
> > *src, size_t n)  {
> > +	uintptr_t dstu = (uintptr_t)dst;
> > +	uintptr_t srcu = (uintptr_t)src;
> >  	void *ret = dst;
> >  	int dstofss;
> >  	int bits;
> > @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
> >  	 */
> >  	if (n < 16) {
> >  		if (n & 0x01) {
> > -			*(uint8_t *)dst = *(const uint8_t *)src;
> > -			src = (const uint8_t *)src + 1;
> > -			dst = (uint8_t *)dst + 1;
> > +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> > +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
> >  		}
> >  		if (n & 0x02) {
> > -			*(uint16_t *)dst = *(const uint16_t *)src;
> > -			src = (const uint16_t *)src + 1;
> > -			dst = (uint16_t *)dst + 1;
> > +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> > +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
> >  		}
> >  		if (n & 0x04) {
> > -			*(uint32_t *)dst = *(const uint32_t *)src;
> > -			src = (const uint32_t *)src + 1;
> > -			dst = (uint32_t *)dst + 1;
> > +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> > +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
> >  		}
> >  		if (n & 0x08) {
> > -			*(uint64_t *)dst = *(const uint64_t *)src;
> > +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
> >  		}
> >  		return ret;
> >  	}
> > @@ -458,6 +460,8 @@ static inline void *  rte_memcpy(void *dst, const
> > void *src, size_t n)  {
> >  	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7,
> xmm8;
> > +	uintptr_t dstu = (uintptr_t)dst;
> > +	uintptr_t srcu = (uintptr_t)src;
> >  	void *ret = dst;
> >  	int dstofss;
> >  	int srcofs;
> > @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
> >  	 */
> >  	if (n < 16) {
> >  		if (n & 0x01) {
> > -			*(uint8_t *)dst = *(const uint8_t *)src;
> > -			src = (const uint8_t *)src + 1;
> > -			dst = (uint8_t *)dst + 1;
> > +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> > +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
> >  		}
> >  		if (n & 0x02) {
> > -			*(uint16_t *)dst = *(const uint16_t *)src;
> > -			src = (const uint16_t *)src + 1;
> > -			dst = (uint16_t *)dst + 1;
> > +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> > +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
> >  		}
> >  		if (n & 0x04) {
> > -			*(uint32_t *)dst = *(const uint32_t *)src;
> > -			src = (const uint32_t *)src + 1;
> > -			dst = (uint32_t *)dst + 1;
> > +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> > +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> > +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
> >  		}
> >  		if (n & 0x08) {
> > -			*(uint64_t *)dst = *(const uint64_t *)src;
> > +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
> >  		}
> >  		return ret;
> >  	}
> > --
> > 1.9.3
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02 12:32   ` Pawel Wodkowski
@ 2015-03-04  5:57     ` Wang, Zhihong
  0 siblings, 0 replies; 10+ messages in thread
From: Wang, Zhihong @ 2015-03-04  5:57 UTC (permalink / raw)
  To: Wodkowski, PawelX, Richardson, Bruce; +Cc: dev



> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Monday, March 02, 2015 8:32 PM
> To: Richardson, Bruce; Wang, Zhihong
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules
> breaking
> 
> On 2015-03-02 11:32, Bruce Richardson wrote:
> > On Mon, Mar 02, 2015 at 05:03:50PM +0800, zhihong.wang@intel.com
> wrote:
> >> Fixed strict-aliasing rules breaking errors for some GCC version.
> >>
> >
> > This looks messy. Also, I believe the definition of memcpy should
> > include the "restrict" keyword to indicate that source and dest can't
> > overlap. Might that help fix the issue?
> >
> 
> Is this error related with overlapping or casting 'void *' to 'uintXX_t *' that
> make compiler report aliasing rule breaking?
> 
> >
> >> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> >> ---
> >>   .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++--
> --------
> >>   1 file changed, 24 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> >> b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> >> index 69a5c6f..f412099 100644
> >> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> >> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> >> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src,
> size_t n)
> >>   static inline void *
> >>   rte_memcpy(void *dst, const void *src, size_t n)
> >>   {
> >> +	uintptr_t dstu = (uintptr_t)dst;
> >> +	uintptr_t srcu = (uintptr_t)src;
> 
> If so maybe using union here would be good solution or 'char *'.

Pawel,

Thanks for the suggestion! But I don't think union can work around this --- already tried in CentOS release 6.5.
Anyway this is for compiler ethics only, the assembly code generated will be the same no matter what kind of method is used.

Zhihong (John)

> 
> --
> Pawel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-04  2:07   ` Wang, Zhihong
@ 2015-03-04 10:18     ` Bruce Richardson
  0 siblings, 0 replies; 10+ messages in thread
From: Bruce Richardson @ 2015-03-04 10:18 UTC (permalink / raw)
  To: Wang, Zhihong; +Cc: dev

On Wed, Mar 04, 2015 at 02:07:20AM +0000, Wang, Zhihong wrote:
> 
> 
> > -----Original Message-----
> > From: Richardson, Bruce
> > Sent: Monday, March 02, 2015 6:32 PM
> > To: Wang, Zhihong
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules
> > breaking
> > 
> > On Mon, Mar 02, 2015 at 05:03:50PM +0800, zhihong.wang@intel.com wrote:
> > > Fixed strict-aliasing rules breaking errors for some GCC version.
> > >
> > 
> > This looks messy. Also, I believe the definition of memcpy should include the
> > "restrict" keyword to indicate that source and dest can't overlap. Might that
> > help fix the issue?
> 
> It's actually caused by casting void * to multiple other pointer types.
> 
Yes, because two pointers of different types are not allowed to point to the
same memory. If the two pointers of different types are belonging to the two
different variables, the "restrict" keyword may indeed help, but that's probably
not the case here.

/Bruce
> > 
> > /Bruce
> > 
> > > Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> > > ---
> > >  .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----
> > ------
> > >  1 file changed, 24 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > > b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > > index 69a5c6f..f412099 100644
> > > --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > > +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> > > @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src,
> > > size_t n)  static inline void *  rte_memcpy(void *dst, const void
> > > *src, size_t n)  {
> > > +	uintptr_t dstu = (uintptr_t)dst;
> > > +	uintptr_t srcu = (uintptr_t)src;
> > >  	void *ret = dst;
> > >  	int dstofss;
> > >  	int bits;
> > > @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
> > >  	 */
> > >  	if (n < 16) {
> > >  		if (n & 0x01) {
> > > -			*(uint8_t *)dst = *(const uint8_t *)src;
> > > -			src = (const uint8_t *)src + 1;
> > > -			dst = (uint8_t *)dst + 1;
> > > +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x02) {
> > > -			*(uint16_t *)dst = *(const uint16_t *)src;
> > > -			src = (const uint16_t *)src + 1;
> > > -			dst = (uint16_t *)dst + 1;
> > > +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x04) {
> > > -			*(uint32_t *)dst = *(const uint32_t *)src;
> > > -			src = (const uint32_t *)src + 1;
> > > -			dst = (uint32_t *)dst + 1;
> > > +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x08) {
> > > -			*(uint64_t *)dst = *(const uint64_t *)src;
> > > +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
> > >  		}
> > >  		return ret;
> > >  	}
> > > @@ -458,6 +460,8 @@ static inline void *  rte_memcpy(void *dst, const
> > > void *src, size_t n)  {
> > >  	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7,
> > xmm8;
> > > +	uintptr_t dstu = (uintptr_t)dst;
> > > +	uintptr_t srcu = (uintptr_t)src;
> > >  	void *ret = dst;
> > >  	int dstofss;
> > >  	int srcofs;
> > > @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
> > >  	 */
> > >  	if (n < 16) {
> > >  		if (n & 0x01) {
> > > -			*(uint8_t *)dst = *(const uint8_t *)src;
> > > -			src = (const uint8_t *)src + 1;
> > > -			dst = (uint8_t *)dst + 1;
> > > +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x02) {
> > > -			*(uint16_t *)dst = *(const uint16_t *)src;
> > > -			src = (const uint16_t *)src + 1;
> > > -			dst = (uint16_t *)dst + 1;
> > > +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x04) {
> > > -			*(uint32_t *)dst = *(const uint32_t *)src;
> > > -			src = (const uint32_t *)src + 1;
> > > -			dst = (uint32_t *)dst + 1;
> > > +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> > > +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> > > +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
> > >  		}
> > >  		if (n & 0x08) {
> > > -			*(uint64_t *)dst = *(const uint64_t *)src;
> > > +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
> > >  		}
> > >  		return ret;
> > >  	}
> > > --
> > > 1.9.3
> > >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02  9:03 [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking zhihong.wang
  2015-03-02 10:32 ` Bruce Richardson
@ 2015-03-05 17:33 ` Thomas Monjalon
  2015-03-06  7:36 ` Liang, Cunming
  2015-03-09  6:14 ` Qiu, Michael
  3 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2015-03-05 17:33 UTC (permalink / raw)
  To: dev

2015-03-02 17:03, zhihong.wang@intel.com:
> Fixed strict-aliasing rules breaking errors for some GCC version.
> 
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
>  .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> index 69a5c6f..f412099 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
>  static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>  	void *ret = dst;
>  	int dstofss;
>  	int bits;
> @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>  	 */
>  	if (n < 16) {
>  		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
[...]

Is there another solution?
Are we going to acknowledge this fix?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02  9:03 [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking zhihong.wang
  2015-03-02 10:32 ` Bruce Richardson
  2015-03-05 17:33 ` Thomas Monjalon
@ 2015-03-06  7:36 ` Liang, Cunming
  2015-03-09 11:44   ` Thomas Monjalon
  2015-03-09  6:14 ` Qiu, Michael
  3 siblings, 1 reply; 10+ messages in thread
From: Liang, Cunming @ 2015-03-06  7:36 UTC (permalink / raw)
  To: zhihong.wang, dev

Hi,

On 3/2/2015 5:03 PM, zhihong.wang@intel.com wrote:
> Fixed strict-aliasing rules breaking errors for some GCC version.
>
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
>   .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
>   1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> index 69a5c6f..f412099 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
>   static inline void *
>   rte_memcpy(void *dst, const void *src, size_t n)
>   {
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>   	void *ret = dst;
>   	int dstofss;
>   	int bits;
> @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>   	 */
>   	if (n < 16) {
>   		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>   		}
>   		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>   		}
>   		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>   		}
>   		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>   		}
>   		return ret;
>   	}
> @@ -458,6 +460,8 @@ static inline void *
>   rte_memcpy(void *dst, const void *src, size_t n)
>   {
>   	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>   	void *ret = dst;
>   	int dstofss;
>   	int srcofs;
> @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>   	 */
>   	if (n < 16) {
>   		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>   		}
>   		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>   		}
>   		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>   		}
>   		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>   		}
>   		return ret;
>   	}

Acked-by:Cunming Liang <cunming.liang@intel.com 
<mailto:cunming.liang@intel.com>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-02  9:03 [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking zhihong.wang
                   ` (2 preceding siblings ...)
  2015-03-06  7:36 ` Liang, Cunming
@ 2015-03-09  6:14 ` Qiu, Michael
  3 siblings, 0 replies; 10+ messages in thread
From: Qiu, Michael @ 2015-03-09  6:14 UTC (permalink / raw)
  To: Wang, Zhihong, dev

On 3/2/2015 5:04 PM, zhihong.wang@intel.com wrote:
> Fixed strict-aliasing rules breaking errors for some GCC version.
>
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---

As this should be a quick fix, this workaround is workable.
Acked-by: Michael Qiu <michael.qiu@intel.com>

>  .../common/include/arch/x86/rte_memcpy.h           | 44 ++++++++++++----------
>  1 file changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> index 69a5c6f..f412099 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
> @@ -195,6 +195,8 @@ rte_mov256blocks(uint8_t *dst, const uint8_t *src, size_t n)
>  static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>  	void *ret = dst;
>  	int dstofss;
>  	int bits;
> @@ -204,22 +206,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>  	 */
>  	if (n < 16) {
>  		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>  		}
>  		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>  		}
>  		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>  		}
>  		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>  		}
>  		return ret;
>  	}
> @@ -458,6 +460,8 @@ static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
>  	__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
> +	uintptr_t dstu = (uintptr_t)dst;
> +	uintptr_t srcu = (uintptr_t)src;
>  	void *ret = dst;
>  	int dstofss;
>  	int srcofs;
> @@ -467,22 +471,22 @@ rte_memcpy(void *dst, const void *src, size_t n)
>  	 */
>  	if (n < 16) {
>  		if (n & 0x01) {
> -			*(uint8_t *)dst = *(const uint8_t *)src;
> -			src = (const uint8_t *)src + 1;
> -			dst = (uint8_t *)dst + 1;
> +			*(uint8_t *)dstu = *(const uint8_t *)srcu;
> +			srcu = (uintptr_t)((const uint8_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint8_t *)dstu + 1);
>  		}
>  		if (n & 0x02) {
> -			*(uint16_t *)dst = *(const uint16_t *)src;
> -			src = (const uint16_t *)src + 1;
> -			dst = (uint16_t *)dst + 1;
> +			*(uint16_t *)dstu = *(const uint16_t *)srcu;
> +			srcu = (uintptr_t)((const uint16_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint16_t *)dstu + 1);
>  		}
>  		if (n & 0x04) {
> -			*(uint32_t *)dst = *(const uint32_t *)src;
> -			src = (const uint32_t *)src + 1;
> -			dst = (uint32_t *)dst + 1;
> +			*(uint32_t *)dstu = *(const uint32_t *)srcu;
> +			srcu = (uintptr_t)((const uint32_t *)srcu + 1);
> +			dstu = (uintptr_t)((uint32_t *)dstu + 1);
>  		}
>  		if (n & 0x08) {
> -			*(uint64_t *)dst = *(const uint64_t *)src;
> +			*(uint64_t *)dstu = *(const uint64_t *)srcu;
>  		}
>  		return ret;
>  	}


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking
  2015-03-06  7:36 ` Liang, Cunming
@ 2015-03-09 11:44   ` Thomas Monjalon
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2015-03-09 11:44 UTC (permalink / raw)
  To: zhihong.wang; +Cc: dev

> > Fixed strict-aliasing rules breaking errors for some GCC version.
> >
> > Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> 
> Acked-by:Cunming Liang <cunming.liang@intel.com 
> <mailto:cunming.liang@intel.com>>

Applied, thanks

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-09 11:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-02  9:03 [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules breaking zhihong.wang
2015-03-02 10:32 ` Bruce Richardson
2015-03-02 12:32   ` Pawel Wodkowski
2015-03-04  5:57     ` Wang, Zhihong
2015-03-04  2:07   ` Wang, Zhihong
2015-03-04 10:18     ` Bruce Richardson
2015-03-05 17:33 ` Thomas Monjalon
2015-03-06  7:36 ` Liang, Cunming
2015-03-09 11:44   ` Thomas Monjalon
2015-03-09  6:14 ` Qiu, Michael

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).