public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: <scott.k.mitch1@gmail.com>, <dev@dpdk.org>
Cc: <stephen@networkplumber.org>, <bruce.richardson@intel.com>,
	<david.marchand@redhat.com>,
	"Cyril Chemparathy" <cchemparathy@ezchip.com>, <stable@dpdk.org>
Subject: RE: [PATCH v18 1/2] eal: add __rte_may_alias and __rte_aligned to unaligned typedefs
Date: Thu, 29 Jan 2026 09:28:02 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35F656C5@smartserver.smartshare.dk> (raw)
In-Reply-To: <20260128194141.90018-2-scott.k.mitch1@gmail.com>

> @@ -199,7 +199,7 @@ verify_jhash_32bits(void)
>  				hash = rte_jhash(key, hashtest_key_lens[i],
>  						hashtest_initvals[j]);
>  				/* Divide key length by 4 in rte_jhash for 32
> bits */
> -				hash32 = rte_jhash_32b((const
> unaligned_uint32_t *)key,
> +				hash32 = rte_jhash_32b((const uint32_t *)key,
>  						hashtest_key_lens[i] >> 2,
>  						hashtest_initvals[j]);
>  				if (hash != hash32) {

rte_jhash_32b() correctly takes a pointer to (aligned) uint32_t, not unaligned, so casting to unaligned might be introducing a bug. (The automatically aligned allocation of the local "key" variable prevents this bug from occurring, but anyway.)
Instead of changing the type cast, I'd prefer fixing this as follows:
Add a local variable uint32_t key32[sizeof(key)/sizeof(uint32_t)], and memcpy(key32,key,sizeof(key)), and then call rte_jhash_32b(key32,...) without type casting.

> +/**
> + * Types for potentially unaligned access.
> + *
> + * __rte_aligned(1) - Reduces alignment requirement to 1 byte,
> allowing
> + *                    these types to safely access memory at any
> address.
> + *                    Without this, accessing a uint16_t at an odd
> address
> + *                    is undefined behavior (even on x86 where
> hardware
> + *                    handles it).
> + *
> + * __rte_may_alias  - Prevents strict-aliasing optimization bugs where
> + *                    compilers may incorrectly elide memory
> operations
> + *                    when casting between pointer types.
> + */
> +#ifdef RTE_TOOLCHAIN_MSVC
> +typedef __rte_may_alias __rte_aligned(1) uint64_t unaligned_uint64_t;
> +typedef __rte_may_alias __rte_aligned(1) uint32_t unaligned_uint32_t;
> +typedef __rte_may_alias __rte_aligned(1) uint16_t unaligned_uint16_t;
> +#else
> +typedef uint64_t unaligned_uint64_t __rte_may_alias __rte_aligned(1);
> +typedef uint32_t unaligned_uint32_t __rte_may_alias __rte_aligned(1);
> +typedef uint16_t unaligned_uint16_t __rte_may_alias __rte_aligned(1);
>  #endif

Skimming GCC documentation, it looks like older versions required placing such attributes after the type, but newer versions seem to recommend placing them before, like qualifiers (const, volatile, ...).
Placing them before the type, like qualifiers, seems more natural to me.
And apparently, MSVC requires it.
Does it work for GCC and Clang if they are placed before, like MSVC?
Then we can get rid of the #ifdef RTE_TOOLCHAIN_MSVC.


  reply	other threads:[~2026-01-29  8:28 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-12 12:04 [PATCH v14 0/2] net: optimize __rte_raw_cksum scott.k.mitch1
2026-01-12 12:04 ` [PATCH v14 1/2] eal: add __rte_may_alias to unaligned typedefs scott.k.mitch1
2026-01-12 13:28   ` Morten Brørup
2026-01-12 15:00     ` Scott Mitchell
2026-01-12 12:04 ` [PATCH v14 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-01-17 21:21 ` [PATCH v15 0/2] net: optimize __rte_raw_cksum scott.k.mitch1
2026-01-17 21:21   ` [PATCH v15 1/2] eal: add __rte_may_alias to unaligned typedefs scott.k.mitch1
2026-01-20 15:23     ` Morten Brørup
2026-01-23 14:34       ` Scott Mitchell
2026-01-17 21:21   ` [PATCH v15 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-01-17 22:08   ` [PATCH v15 0/2] net: optimize __rte_raw_cksum Stephen Hemminger
2026-01-20 12:45     ` Morten Brørup
2026-01-23 15:43       ` Scott Mitchell
2026-01-23 16:02   ` [PATCH v16 " scott.k.mitch1
2026-01-23 16:02     ` [PATCH v16 1/2] eal: add __rte_may_alias to unaligned typedefs scott.k.mitch1
2026-01-23 16:02     ` [PATCH v16 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-01-28 11:05       ` David Marchand
2026-01-28 17:39         ` Scott Mitchell
2026-01-24  8:23     ` [PATCH v16 0/2] net: optimize __rte_raw_cksum Morten Brørup
2026-01-28 18:05     ` [PATCH v17 " scott.k.mitch1
2026-01-28 18:05       ` [PATCH v17 1/2] eal: add __rte_may_alias and __rte_aligned to unaligned typedefs scott.k.mitch1
2026-01-28 18:05       ` [PATCH v17 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-01-28 19:41       ` [PATCH v18 0/2] net: optimize __rte_raw_cksum scott.k.mitch1
2026-01-28 19:41         ` [PATCH v18 1/2] eal: add __rte_may_alias and __rte_aligned to unaligned typedefs scott.k.mitch1
2026-01-29  8:28           ` Morten Brørup [this message]
2026-02-02  4:31             ` Scott Mitchell
2026-01-28 19:41         ` [PATCH v18 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-01-29  8:31           ` Morten Brørup
2026-02-02  4:48         ` [PATCH v19 0/2] net: optimize __rte_raw_cksum scott.k.mitch1
2026-02-02  4:48           ` [PATCH v19 1/2] eal: add __rte_may_alias and __rte_aligned to unaligned typedefs scott.k.mitch1
2026-02-03  8:18             ` Morten Brørup
2026-02-16 14:29             ` David Marchand
2026-02-16 15:00               ` Morten Brørup
2026-02-02  4:48           ` [PATCH v19 2/2] net: __rte_raw_cksum pointers enable compiler optimizations scott.k.mitch1
2026-02-03  8:19             ` Morten Brørup
2026-02-06 14:54           ` [PATCH v19 0/2] net: optimize __rte_raw_cksum David Marchand
2026-02-07  1:29             ` Scott Mitchell
2026-02-10 11:53               ` Thomas Monjalon
2026-02-16 14:04               ` David Marchand

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=98CBD80474FA8B44BF855DF32C47DC35F656C5@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=bruce.richardson@intel.com \
    --cc=cchemparathy@ezchip.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=scott.k.mitch1@gmail.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    /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