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

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

On Wed, Mar 11, 2026 at 07:29:38PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Wednesday, 11 March 2026 17.59
> > 
> > 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.
> 
> Agree!
> 
> I just counted; 149 of the added lines are for handling __rte_constant(n). So it's not as bad as it looks.
> But still growing, which was not the intention.
> When I started working on this patch, the intention was to consolidate the copy-pasted instances for handling up to 64 bytes into one instance. This should have reduced the amount of code.
> But then it somehow grew anyway.
> 
> > 
> > > 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..."
> 
> The comment is still present in the "generic" header file used for the Doxygen documentation:
> https://elixir.bootlin.com/dpdk/v26.03-rc1/source/lib/eal/include/generic/rte_memcpy.h#L99
> 
> All other architectures rely on the "generic" header file, and have no Doxygen comments at all.
> We could also remove them from the x86 implementation.
> That would shrink the file even more. ;-)
> But I'd rather keep the comments - at least for now.
> 
> > 
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Thank you for quick response, Bruce.
> 
> > 
> > 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!
> 
> Good idea. But let's not do it now.
>
Agree on all counts.