From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 58259C884 for ; Fri, 19 Jun 2015 17:57:56 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1Z5ykY-0007Ce-8a; Fri, 19 Jun 2015 18:02:43 +0200 Message-ID: <55843C41.7010702@6wind.com> Date: Fri, 19 Jun 2015 17:58:57 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: Cyril Chemparathy , dev@dpdk.org References: <1430324134-25654-1-git-send-email-cchemparathy@ezchip.com> <1430324134-25654-2-git-send-email-cchemparathy@ezchip.com> In-Reply-To: <1430324134-25654-2-git-send-email-cchemparathy@ezchip.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 01/10] eal: add and use unaligned integer types X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jun 2015 15:57:56 -0000 Hello Cyril, First, sorry commenting that late. My first intent was to benchmark with your modifications, but I did not find the time for it. So, please find some comments on your series below so it can be pushed for 2.1. On 04/29/2015 06:15 PM, Cyril Chemparathy wrote: > On machines that are strict on pointer alignment, current code breaks on GCC's > -Wcast-align checks on casts from narrower to wider types. This patch > introduces new unaligned_uint(16|32|64)_t types, which correctly retain > alignment in such cases. > > This is currently unimplemented on ICC and clang, and equivalents will need to > be plugged in. > > Signed-off-by: Cyril Chemparathy > --- > app/test-pmd/flowgen.c | 4 ++-- > app/test-pmd/txonly.c | 2 +- > app/test/packet_burst_generator.c | 4 ++-- > app/test/test_mbuf.c | 16 ++++++++-------- > lib/librte_eal/common/include/rte_common.h | 10 ++++++++++ > lib/librte_ether/rte_ether.h | 2 +- > lib/librte_ip_frag/rte_ipv4_reassembly.c | 4 ++-- > 7 files changed, 26 insertions(+), 16 deletions(-) > > [...] > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h > index c0ab8b4..3bb97d1 100644 > --- a/lib/librte_eal/common/include/rte_common.h > +++ b/lib/librte_eal/common/include/rte_common.h > @@ -61,6 +61,16 @@ extern "C" { > > /*********** Macros to eliminate unused variable warnings ********/ > > +#if defined(__GNUC__) > +typedef uint64_t unaligned_uint64_t __attribute__ ((aligned(1))); > +typedef uint32_t unaligned_uint32_t __attribute__ ((aligned(1))); > +typedef uint16_t unaligned_uint16_t __attribute__ ((aligned(1))); > +#else > +typedef uint64_t unaligned_uint64_t; > +typedef uint32_t unaligned_uint32_t; > +typedef uint16_t unaligned_uint16_t; > +#endif > + Shouldn't we only define these unaligned types for architectures that have strict alignment constraints ? I am a bit puzzled by only defining it when compiling with gcc: is it because only gcc triggers a warning? If yes, I'm not sure it's a good reason. Maybe we could have a compile-time option to enable these types, and this option would be set for architectures that require it only. The advantage would be to ensure there's no modification on x86. What do you think? cosmetic: the definitions should go above the comment line that refers to the macro below. For the rest of the series (except a small comment on patch 8/10, see the other mail): Acked-by: Olivier Matz