From: Cyril Chemparathy <cchemparathy@ezchip.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 01/10] eal: add and use unaligned integer types
Date: Fri, 19 Jun 2015 10:18:17 -0700 [thread overview]
Message-ID: <20150619101817.4884a947@cchemparathy-ubuntu> (raw)
In-Reply-To: <55843C41.7010702@6wind.com>
On Fri, 19 Jun 2015 17:58:57 +0200
Olivier MATZ <olivier.matz@6wind.com> wrote:
> 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 <cchemparathy@ezchip.com>
> > ---
> > 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?
>
Fair enough. I like that better than keying off of GCC or anything
like that. I will change this patch accordingly for the next revision.
> 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 <olivier.matz@6wind.com>
>
>
next prev parent reply other threads:[~2015-06-19 17:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-29 16:15 [dpdk-dev] [PATCH 00/10] Improve cast alignment for strict aligned machines Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 01/10] eal: add and use unaligned integer types Cyril Chemparathy
2015-06-19 15:58 ` Olivier MATZ
2015-06-19 17:18 ` Cyril Chemparathy [this message]
2015-04-29 16:15 ` [dpdk-dev] [PATCH 02/10] mempool: silence -Wcast-align on pointer arithmetic Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 03/10] hash: " Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 04/10] mbuf: " Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 05/10] ethdev: " Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 06/10] app/test-pmd: pack simple_gre_hdr Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 07/10] app/test: use struct ether_addr instead of a byte array cast Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 08/10] librte_mbuf: Add rte_pktmbuf_mtod_offset() Cyril Chemparathy
2015-06-19 15:58 ` Olivier MATZ
2015-06-19 17:18 ` Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 09/10] librte_mbuf: Add transform for rte_pktmbuf_mtod_offset() Cyril Chemparathy
2015-04-29 16:15 ` [dpdk-dev] [PATCH 10/10] librte_mbuf: Apply mtod-offset.cocci transform Cyril Chemparathy
2015-05-04 9:50 ` [dpdk-dev] [PATCH 00/10] Improve cast alignment for strict aligned machines Olivier MATZ
2015-05-04 17:26 ` Cyril Chemparathy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150619101817.4884a947@cchemparathy-ubuntu \
--to=cchemparathy@ezchip.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).