Re: [PATCH v7] eal/x86: optimize memcpy of small sizes

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Feb 20, 2026 at 11:08:24AM +0000, Morten Brørup wrote:
> The implementation for copying up to 64 bytes does not depend on address
> alignment with the size of the CPU's vector registers. Nonetheless, the
> exact same code for copying up to 64 bytes was present in both the aligned
> copy function and all the CPU vector register size specific variants of
> the unaligned copy functions.
> With this patch, the implementation for copying up to 64 bytes was
> consolidated into one instance, located in the common copy function,
> before checking alignment requirements.
> This provides three benefits:
> 1. No copy-paste in the source code.
> 2. A performance gain for copying up to 64 bytes, because the
> address alignment check is avoided in this case.
> 3. Reduced instruction memory footprint, because the compiler only
> generates one instance of the function for copying up to 64 bytes, instead
> of two instances (one in the unaligned copy function, and one in the
> aligned copy function).
> 
> Furthermore, the function for copying less than 16 bytes was replaced with
> a smarter implementation using fewer branches and potentially fewer
> load/store operations.
> This function was also extended to handle copying of up to 16 bytes,
> instead of up to 15 bytes.
> This small extension reduces the code path, and thus improves the
> performance, for copying two pointers on 64-bit architectures and four
> pointers on 32-bit architectures.
> 
> Also, __rte_restrict was added to source and destination addresses.
> 
> And finally, the missing implementation of rte_mov48() was added.
> 
> Regarding performance, the memcpy performance test showed cache-to-cache
> copying of up to 32 bytes now takes 2 cycles, versus ca. 6.5 cycles before
> this patch.
> Copying 64 bytes now takes 4 cycles, versus 7 cycles before.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> v7:
> * Updated patch description. Mainly to clarify that the changes related to
>   copying up to 64 bytes simply replaces multiple instances of copy-pasted
>   code with one common instance.
> * Fixed copy of build time known 16 bytes in rte_mov17_to_32(). (Vipin)
> * Rebased.
> v6:
> * Went back to using rte_uintN_alias structures for copying instead of
>   using memcpy(). They were there for a reason.
>   (Inspired by the discussion about optimizing the checksum function.)
> * Removed note about copying uninitialized data.
> * Added __rte_restrict to source and destination addresses.
>   Updated function descriptions from "should" to "must" not overlap.
> * Changed rte_mov48() AVX implementation to copy 32+16 bytes instead of
>   copying 32 + 32 overlapping bytes. (Konstantin)
> * Ignoring "-Wstringop-overflow" is not needed, so it was removed.
> v5:
> * Reverted v4: Replace SSE2 _mm_loadu_si128() with SSE3 _mm_lddqu_si128().
>   It was slower.
> * Improved some comments. (Konstantin Ananyev)
> * Moved the size range 17..32 inside the size <= 64 branch, so when
>   building for SSE, the generated code can start copying the first
>   16 bytes before comparing if the size is greater than 32 or not.
> * Just require RTE_MEMCPY_AVX for using rte_mov32() in rte_mov33_to_64().
> v4:
> * Replace SSE2 _mm_loadu_si128() with SSE3 _mm_lddqu_si128().
> v3:
> * Fixed typo in comment.
> v2:
> * Updated patch title to reflect that the performance is improved.
> * Use the design pattern of two overlapping stores for small copies too.
> * Expanded first branch from size < 16 to size <= 16.
> * Handle more build time constant copy sizes.
> ---
>  lib/eal/x86/include/rte_memcpy.h | 526 ++++++++++++++++++++-----------
>  1 file changed, 348 insertions(+), 178 deletions(-)
> 

I'm a little unhappy to see the amount of memcpy code growing rather than
shrinking, but since it improves performance I'm ok with it. We should keep
it under constant review though.

> diff --git a/lib/eal/x86/include/rte_memcpy.h b/lib/eal/x86/include/rte_memcpy.h
> index 46d34b8081..ed8e5f8dc4 100644
> --- a/lib/eal/x86/include/rte_memcpy.h
> +++ b/lib/eal/x86/include/rte_memcpy.h
> @@ -22,11 +22,6 @@
>  extern "C" {
>  #endif
>  
> -#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100000)
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wstringop-overflow"
> -#endif
> -
>  /*
>   * GCC older than version 11 doesn't compile AVX properly, so use SSE instead.
>   * There are no problems with AVX2.
> @@ -40,9 +35,6 @@ extern "C" {
>  /**
>   * Copy bytes from one location to another. The locations must not overlap.
>   *
> - * @note This is implemented as a macro, so it's address should not be taken
> - * and care is needed as parameter expressions may be evaluated multiple times.
> - *

I'd be wary about completely removing this comment, as we may well want to
go back to a macro in the future, e.g. if we decide to remove the custom
rte_memcpy altogether. Therefore, rather than removing the comment, can we
tweak it to say "This may be implemented as a macro..."


Acked-by: Bruce Richardson <bruce.richardson@intel.com>

PS: If we want a little further cleanup, I'd consider removing the
RTE_MEMCPY_AVX macro and replacing it with a straight check for __AVX2__.
CPUs with AVX2 was introduced in 2013, and checking Claude and Wikipedia
says that AMD parts started having it in 2015, meaning that there were only
a few generations of CPUs >10 years ago which had AVX but not AVX2. [There
were later CPUs e.g. lower-end parts, which didn't have AVX2, but they
didn't have AVX1 either, so SSE is the only choice there]
Not a big cleanup if we did remove it, but sometimes every little helps!