From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bruce.richardson@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id D19DF5683
 for <dev@dpdk.org>; Wed,  4 Mar 2015 11:18:05 +0100 (CET)
Received: from orsmga003.jf.intel.com ([10.7.209.27])
 by fmsmga102.fm.intel.com with ESMTP; 04 Mar 2015 02:18:03 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.09,686,1418112000"; d="scan'208";a="535886061"
Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.24])
 by orsmga003.jf.intel.com with SMTP; 04 Mar 2015 02:17:58 -0800
Received: by  (sSMTP sendmail emulation); Wed, 04 Mar 2015 10:18:00 +0025
Date: Wed, 4 Mar 2015 10:18:00 +0000
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Wang, Zhihong" <zhihong.wang@intel.com>
Message-ID: <20150304101800.GC1468@bricha3-MOBL3>
References: <1425287030-18225-1-git-send-email-zhihong.wang@intel.com>
 <20150302103224.GC8520@bricha3-MOBL3>
 <F60F360A2500CD45ACDB1D700268892D0E790DF6@SHSMSX101.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <F60F360A2500CD45ACDB1D700268892D0E790DF6@SHSMSX101.ccr.corp.intel.com>
Organization: Intel Shannon Ltd.
User-Agent: Mutt/1.5.23 (2014-03-12)
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] A fix to work around strict-aliasing rules
 breaking
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 04 Mar 2015 10:18:06 -0000

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