From: "Morten Brørup" <mb@smartsharesystems.com>
To: <dev@dpdk.org>, "Bruce Richardson" <bruce.richardson@intel.com>
Cc: "Stephen Hemminger" <stephen@networkplumber.org>
Subject: RE: [PATCH] eal/x86: optimize memcpy of small 64-byte blocks
Date: Tue, 4 Aug 2026 21:11:51 +0200 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35F659BB@smartserver.smartshare.dk> (raw)
In-Reply-To: <20260804143305.1344327-1-mb@smartsharesystems.com>
> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Tuesday, 4 August 2026 16.33
>
> 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>
> ---
The AI in the CI is confused, so the AI Code Review warning in Patchwork can be ignored.
Quoting and commenting AI code review:
> ### 1. Bitwise test incorrect for 512-byte blocks
>
> **File:** `lib/eal/x86/include/rte_memcpy.h:712`
>
> ```c
> if (n & 512) {
> ```
>
> **Issue:**
> This test is wrong.
> `n & 512` is true when bit 9 is set, which occurs for n=512, n=513, ..., n=767, and many other values outside the intended range.
> For a size of exactly 512 bytes, you want `n == 512` or `n >= 512` (with an upper bound already enforced by the outer `if`).
Rubbish. Combined with the overall criteria for entering this at all, the test matches 512 exactly as intended.
>
> **Why it matters:**
> With `n=128`, the expression `128 & 512` evaluates to 0 (correct, no copy).
> But with `n=640` (which is > `RTE_MEMCPY_BLOCK_64_MAX` on AVX and shouldn't reach this code on AVX512),
> if it did reach here, `640 & 512 = 512` would be true and trigger a 512-byte copy when only 640 bytes total are requested.
Irrelevant. It doesn't come here when n > 512.
>
> More critically, the surrounding checks are also bitwise tests (`n & 256`, `n & 128`, `n & 64`),
> which means this code assumes `n` is a combination of these powers of two.
> But the entry condition is `n <= RTE_MEMCPY_BLOCK_64_MAX` and `(n & 63) == 0`, which allows any multiple of 64 up to the max (64, 128, 192, 256, 320, 384, 448, 512).
Correct.
> For `n=192`, all four bitwise tests fail and nothing is copied.
Wrong. 192 = 128 + 64, so both of those will execute.
> For `n=320`, `n & 256 = 256` is true so it copies 256 bytes, then `n & 64 = 64` is true so it copies another 64 bytes from offset 256, which is correct only by accident (320 = 256+64).
Not by accident, by design.
next prev parent reply other threads:[~2026-08-04 19:11 UTC|newest]
Thread overview: 11+ 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-04 19:11 ` Morten Brørup [this message]
2026-08-04 20:42 ` Stephen Hemminger
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=98CBD80474FA8B44BF855DF32C47DC35F659BB@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=stephen@networkplumber.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox