> > + 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.
Sounds good, fix coming in v19.
> > +/**
> > + * 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.
Good call! https://godbolt.org/z/oYrnfsMM3 gcc 8 and clang 7 both
support attributes before the type.
