All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Konstantin Ananyev" <konstantin.ananyev@huawei.com>,
	<dev@dpdk.org>, "Bruce Richardson" <bruce.richardson@intel.com>
Subject: RE: [PATCH] eal/x86: optimize memcpy of small 64-byte blocks
Date: Wed, 5 Aug 2026 22:16:02 +0200	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35F659C2@smartserver.smartshare.dk> (raw)
In-Reply-To: <cde9ae69207e4a8fb1b0b50d8b7b7539@huawei.com>

> From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com]
> Sent: Wednesday, 5 August 2026 19.10
> 
> > The implementation for copying 64-byte blocks up to 512 (or 256)
> bytes
> > does not depend on address alignment with the size of the CPU's
> vector
> > registers, but is implemented in both unaligned and aligned copy
> > functions.
> > The main rte_memcpy() function was updated, so
> > if the copy size is known at compile time and the other criteria
> match,
> > the copy is performed without checking alignment requirements.
> > This provides two benefits when the optimization comes into play:
> > 1. A performance gain, because the address alignment check is
> avoided.
> > 3. Reduced instruction memory footprint, because the compiler only
> > generates one instance of the function for copying, instead of two
> > instances (one in the unaligned copy function, and one in the aligned
> > copy function).
> >
> > Furthermore, the temporary alignment mask definition (ALIGNMENT_MASK)
> > was prefixed by RTE_MEMCPY_ to prevent potential namespace collision.
> >
> > And finally, the superfluous function declaration at the top of the
> > file was removed, and its description was moved to the function
> > definition.
> > This improves search results with source code browsers.
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > ---
> >  lib/eal/x86/include/rte_memcpy.h | 64 ++++++++++++++++++++++--------
> --
> >  1 file changed, 44 insertions(+), 20 deletions(-)
> >
> > diff --git a/lib/eal/x86/include/rte_memcpy.h
> > b/lib/eal/x86/include/rte_memcpy.h
> > index 8ed8c55010..ce5a55c36f 100644
> > --- a/lib/eal/x86/include/rte_memcpy.h
> > +++ b/lib/eal/x86/include/rte_memcpy.h
> > @@ -32,21 +32,6 @@ extern "C" {
> >  #define RTE_MEMCPY_AVX
> >  #endif
> >
> > -/**
> > - * Copy bytes from one location to another. The locations must not
> overlap.
> > - *
> > - * @param dst
> > - *   Pointer to the destination of the data.
> > - * @param src
> > - *   Pointer to the source data.
> > - * @param n
> > - *   Number of bytes to copy.
> > - * @return
> > - *   Pointer to the destination data.
> > - */
> > -static __rte_always_inline void *
> > -rte_memcpy(void *__rte_restrict dst, const void *__rte_restrict src,
> size_t n);
> > -
> >  /**
> >   * Copy bytes from one location to another,
> >   * locations must not overlap.
> > @@ -187,7 +172,8 @@ rte_mov256(uint8_t *__rte_restrict dst, const
> uint8_t
> > *__rte_restrict src)
> >   * AVX512 implementation below
> >   */
> >
> > -#define ALIGNMENT_MASK 0x3F
> > +#define RTE_MEMCPY_ALIGNMENT_MASK 0x3F
> > +#define RTE_MEMCPY_BLOCK_64_MAX 512
> >
> >  /**
> >   * Copy 128-byte blocks from one location to another,
> > @@ -333,7 +319,8 @@ rte_memcpy_generic_more_than_64(void
> *__rte_restrict
> > dst, const void *__rte_rest
> >   * AVX implementation below
> >   */
> >
> > -#define ALIGNMENT_MASK 0x1F
> > +#define RTE_MEMCPY_ALIGNMENT_MASK 0x1F
> > +#define RTE_MEMCPY_BLOCK_64_MAX 256
> 
> Wonder why BLOCK_64_MAX is 512 for SSE and AVX512, but 256 for AVX2?
> Some empirical data or ...?

I wonder too.
Intel wrote it that way.
I considered consolidating on 512 for simplicity.
But then I would be changing the algorithm without any data to support the change.

Now you tickled my curiosity, so I went hunting...
It was 512 until DPDK 16.04, but in 16.07 it was changed to 256 with this patch:
https://github.com/DPDK/dpdk/commit/4b42e90ef0e421dc777f2b2e377eb237cd3675fa

> 
> >
> >  /**
> >   * Copy 128-byte blocks from one location to another,
> > @@ -444,7 +431,8 @@ rte_memcpy_generic_more_than_64(void
> *__rte_restrict
> > dst, const void *__rte_rest
> >   * SSE implementation below
> >   */
> >
> > -#define ALIGNMENT_MASK 0x0F
> > +#define RTE_MEMCPY_ALIGNMENT_MASK 0x0F
> > +#define RTE_MEMCPY_BLOCK_64_MAX 512
> >
> >  /**
> >   * Macro for copying unaligned block from one location to another
> with
> > constant load offset,
> > @@ -673,6 +661,18 @@ rte_memcpy_aligned_more_than_64(void
> > *__rte_restrict dst, const void *__rte_rest
> >  	return ret;
> >  }
> >
> > +/**
> > + * Copy bytes from one location to another. The locations must not
> overlap.
> > + *
> > + * @param dst
> > + *   Pointer to the destination of the data.
> > + * @param src
> > + *   Pointer to the source data.
> > + * @param n
> > + *   Number of bytes to copy.
> > + * @return
> > + *   Pointer to the destination data.
> > + */
> >  static __rte_always_inline void *
> >  rte_memcpy(void *__rte_restrict dst, const void *__rte_restrict src,
> size_t n)
> >  {
> > @@ -707,15 +707,39 @@ rte_memcpy(void *__rte_restrict dst, const void
> > *__rte_restrict src, size_t n)
> >  #endif
> >  		return dst;
> >  	}
> > +	/* Common way for small copy size of 64-byte blocks. Unlikely, so
> > constant size only */
> > +	if (__rte_constant(n) && (n & 63) == 0 && n <=
> > RTE_MEMCPY_BLOCK_64_MAX) {
> 
> You probably need a check (static_assert?) that RTE_MEMCPY_BLOCK_64_MAX
> is
> Within allowed values (<= 512+256+128+64).
> Otherwise code block below might not be enough to copy everything.

Allowed values stop at 512. Note that src and dst are not incremented for 512.

I would hope that someone changing the RTE_MEMCPY_BLOCK_64_MAX definition (or introducing a new value if support for a new vector register length is added) doesn't do it without considering what it is used for. A static_assert seems overkill here.

> 
> > +		void *ret = dst;
> > +
> > +		if (n & 512) {
> > +			rte_mov256((uint8_t *)dst + 0 * 256, (const uint8_t
> *)src
> > + 0 * 256);
> > +			rte_mov256((uint8_t *)dst + 1 * 256, (const uint8_t
> *)src
> > + 1 * 256);
> > +		}
> > +		if (n & 256) {
> > +			rte_mov256((uint8_t *)dst, (const uint8_t *)src);
> > +			src = (const uint8_t *)src + 256;
> > +			dst = (uint8_t *)dst + 256;
> > +		}
> > +		if (n & 128) {
> > +			rte_mov128((uint8_t *)dst, (const uint8_t *)src);
> > +			src = (const uint8_t *)src + 128;
> > +			dst = (uint8_t *)dst + 128;
> > +		}
> > +		if (n & 64)
> > +			rte_mov64((uint8_t *)dst, (const uint8_t *)src);
> > +
> > +		return ret;
> > +	}
> >
> >  	/* Implementation for size > 64 bytes depends on alignment with
> vector
> > register size. */
> > -	if (!(((uintptr_t)dst | (uintptr_t)src) & ALIGNMENT_MASK))
> > +	if (!(((uintptr_t)dst | (uintptr_t)src) &
> RTE_MEMCPY_ALIGNMENT_MASK))
> >  		return rte_memcpy_aligned_more_than_64(dst, src, n);
> >  	else
> >  		return rte_memcpy_generic_more_than_64(dst, src, n);
> >  }
> >
> > -#undef ALIGNMENT_MASK
> > +#undef RTE_MEMCPY_ALIGNMENT_MASK
> > +#undef RTE_MEMCPY_BLOCK_64_MAX
> >
> >  #ifdef __cplusplus
> >  }
> > --
> > 2.43.0


  reply	other threads:[~2026-08-05 20:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 14:33 [PATCH] eal/x86: optimize memcpy of small 64-byte blocks Morten Brørup
2026-08-04 14:33 ` [RFC PATCH] pile stack and mempool driver (resend) Morten Brørup
2026-08-04 14:38   ` Morten Brørup
2026-08-04 15:52 ` [PATCH] eal/x86: optimize memcpy of small 64-byte blocks Stephen Hemminger
2026-08-04 16:25   ` Morten Brørup
2026-08-05  5:45     ` Konstantin Ananyev
2026-08-05  5:56       ` Morten Brørup
2026-08-05  6:50         ` Konstantin Ananyev
2026-08-05  8:36           ` Morten Brørup
2026-08-05 17:16             ` Konstantin Ananyev
2026-08-05 20:28               ` Morten Brørup
2026-08-06  7:58                 ` Konstantin Ananyev
2026-08-06  9:17                   ` Morten Brørup
2026-08-04 19:11 ` Morten Brørup
2026-08-04 20:42   ` Stephen Hemminger
2026-08-05 17:10 ` Konstantin Ananyev
2026-08-05 20:16   ` Morten Brørup [this message]
2026-08-06  7:43     ` Konstantin Ananyev
2026-08-06  8:00       ` Morten Brørup
2026-08-06 10:17 ` [PATCH v2] " Morten Brørup

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=98CBD80474FA8B44BF855DF32C47DC35F659C2@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@huawei.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.