> > > - /**
> > > - * Use the following structs to avoid violating C standard
> > > - * alignment requirements and to avoid strict aliasing bugs
> > > - */
> > > - struct __rte_packed_begin rte_uint64_alias {
> > > - uint64_t val;
> > > - } __rte_packed_end __rte_may_alias;
> > > - struct __rte_packed_begin rte_uint32_alias {
> > > - uint32_t val;
> > > - } __rte_packed_end __rte_may_alias;
> > > - struct __rte_packed_begin rte_uint16_alias {
> > > - uint16_t val;
> > > - } __rte_packed_end __rte_may_alias;
The discussion about the optimized checksum function [1] has shown us that
memcpy() sometimes prevents Clang from optimizing (loop unrolling and
vectorizing) and potentially causes strict aliasing bugs with GCC, so I will
work on a new patch version that keeps using the above types, instead of
introducing memcpy() inside rte_memcpy().
[1]:
https://inbox.dpdk.org/dev/CAFn2buBzBLFLVN-K=u3mgbebq-hqbgjlvpdx3vsxvkjpa0y...@mail.gmail.com/
> > > +static __rte_always_inline void
> > > +rte_mov48(uint8_t *dst, const uint8_t *src)
> > > +{
> > > +#if defined RTE_MEMCPY_AVX
> > > + rte_mov32((uint8_t *)dst, (const uint8_t *)src);
> > > + rte_mov32((uint8_t *)dst - 32 + 48, (const uint8_t *)src - 32 +
> > > 48);
>
> Just a s thought: would compiler and CPU be smart enough to realize
> that there is no dependency between these 2 ops, and they can be
> executed in any
> order?
> Might be do mov32(); mov16() instead?
> Again' didn't test anything, just a thought.
Good idea.
I simply copied what the existing AVX code did for copying 48 bytes, but I
agree with your suggestion.
>
> > > +#else /* SSE implementation */
> > > + rte_mov16((uint8_t *)dst + 0 * 16, (const uint8_t *)src + 0 *
> > > 16);
> > > + rte_mov16((uint8_t *)dst + 1 * 16, (const uint8_t *)src + 1 *
> > > 16);
> > > + rte_mov16((uint8_t *)dst + 2 * 16, (const uint8_t *)src + 2 *
> > > 16);
> > > +#endif
> > > +}
> > > +
> > > /**
> > > * Copy 64 bytes from one location to another,
> > > * locations should not overlap.
> > > @@ -172,6 +143,137 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
> > > rte_mov128(dst + 1 * 128, src + 1 * 128);
> > > }
> > >
> > > +/**
> > > + * Copy bytes from one location to another,
> > > + * locations should not overlap.
> > > + * Use with n <= 16.
> > > + *
> > > + * Note: Copying uninitialized memory is perfectly acceptable.
> > > + * Using e.g. memcpy(dst, src, 8) instead of
> > > + * *(unaligned_uint64_t*) = *(const unaligned_uint64_t *)src
> > > + * avoids compiler warnings about source data may be uninitialized
> > > + * [-Wmaybe-uninitialized].
> > > + */
> > > +static __rte_always_inline void *
> > > +rte_mov16_or_less(void *dst, const void *src, size_t n)
> > > +{
> > > + /* Faster way when size is known at build time. */
> > > + if (__rte_constant(n)) {
> > > + if (n == 2)
> > > + return memcpy(dst, src, 2);
> > > + if (n == 4)
> > > + return memcpy(dst, src, 4);
> > > + if (n == 6) /* 4 + 2 */
> > > + return memcpy(dst, src, 6);
> > > + if (n == 8)
> > > + return memcpy(dst, src, 8);
> > > + if (n == 10) /* 8 + 2 */
> > > + return memcpy(dst, src, 10);
> > > + if (n == 12) /* 8 + 4 */
> > > + return memcpy(dst, src, 12);
> > > + if (n == 16) {
> > > + rte_mov16((uint8_t *)dst, (const uint8_t *)src);
> > > + return dst;
> > > + }
>
> If n is constant; wouldn't compiler unroll such memcpy itself?
> Specially for such small (<=16) values?
> I mean. can't we just:
> If (n < 16) memcpy(dst, src, n); else rte_mov16(dst, src);
Unfortunately not. For e.g. n == 13, we want to use the trick with the
overlapping copies, requiring only two 8-byte copy operations instead of three
copy operations (8-byte + 4-byte + 1-byte).
>
> > > + }
> > > +
> > > + /*
> > > + * Note: Using "n & X" generates 3-byte "test" instructions,
> > > + * instead of "n >= X", which would generate 4-byte "cmp"
> > > instructions.
> > > + */
> > > + if (n & 0x18) { /* n >= 8, including n == 0x10, hence n & 0x18.
> > > */
> > > + /* Copy 8 ~ 16 bytes. */
> > > + memcpy(dst, src, 8);
> > > + memcpy((uint8_t *)dst - 8 + n, (const uint8_t *)src - 8 +
> > > n, 8);
> > > + } else if (n & 0x4) {
> > > + /* Copy 4 ~ 7 bytes. */
> > > + memcpy(dst, src, 4);
> > > + memcpy((uint8_t *)dst - 4 + n, (const uint8_t *)src - 4 +
> > > n, 4);
> > > + } else if (n & 0x2) {
> > > + /* Copy 2 ~ 3 bytes. */
> > > + memcpy(dst, src, 2);
> > > + memcpy((uint8_t *)dst - 2 + n, (const uint8_t *)src - 2 +
> > > n, 2);
> > > + } else if (n & 0x1) {
> > > + /* Copy 1 byte. */
> > > + memcpy(dst, src, 1);
> > > + }
> > > + return dst;
> > > +}
