[RFC 0/1] replace inline assembly prefetch with cc builtins

[RFC 0/1] replace inline assembly prefetch with cc builtins

[Mattias Rönnblom]

This is an unorthodox RFC in that I think it probably shouldn't be
accepted.

I thought I'd try replacing the x86 inline assembly for DPDK software
prefetch with GCC __builtin_prefetch().

Being essentially a black box for the compiler, inline assembly
hinders certain optimizations. The hope was beyond DPDK source code
simplification there would also be a performance upside.

I evaluate this change in an eventdev-based application with a
multi-stage pipeline, which heavily relies on software prefetching to
mitigate the cost of core-to-core transitions. After I replaced the
lib/eal/x86/include/rte_prefetch.h inline assembly with calls to GCC
built-ins, the reference application saw a ~50 cc/stage slowdown on
GCC 13.3.0 and GCC 14.2.0.

So unfortunately GCC uses its newfound freedom to make the code
slower. I'm guessing it reorders the prefetches to happen at a
too-late time.

On clang 17.0.6 and clang 18.1.3, the application-level performance
remains the same after the change.

I've tested only a single application, so it would be of great value
of we could get some more experience (preferably from real-world
apps).

rte_prefetch*_write() is using GCC built-ins, so I tried moving that
to inline assembly. Initially a saw a 10 cc/stage gain from this move
(on GCC), but after reorganizing the prefetch logic, that gain was
gone. I have a patch for that change as well, if anyone is
interested. (It's a little more complicated that you might think,
since you need to detect if prefetchw is available in the target ISA.)

Mattias Rönnblom (1):
  eal/x86: replace inline assembly prefetch with cc builtins

 lib/eal/x86/include/rte_prefetch.h | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

-- 
2.43.0

[RFC 1/1] eal/x86: replace inline assembly prefetch with cc builtins

[Mattias Rönnblom]

Implement the 32- and 64-bit x86 rte_prefetch<n>() functions and
rte_prefetch_non_temporal() using compiler builtins, rather than
inline assembly.

This change frees the compiler to do certain optimizations that
otherwise wouldn't be possible.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 lib/eal/x86/include/rte_prefetch.h | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/lib/eal/x86/include/rte_prefetch.h b/lib/eal/x86/include/rte_prefetch.h
index 34a609cc65..329b1a81f7 100644
--- a/lib/eal/x86/include/rte_prefetch.h
+++ b/lib/eal/x86/include/rte_prefetch.h
@@ -22,7 +22,11 @@ static inline void rte_prefetch0(const volatile void *p)
 #ifdef RTE_TOOLCHAIN_MSVC
 	_mm_prefetch((const char *)(uintptr_t)p, _MM_HINT_T0);
 #else
-	asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char *)p));
+	/* 0 indicates intention to read, 3 sets target cache level to L1. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch((const void *)(uintptr_t)p, 0, 3);
 #endif
 }
 
@@ -31,7 +35,11 @@ static inline void rte_prefetch1(const volatile void *p)
 #ifdef RTE_TOOLCHAIN_MSVC
 	_mm_prefetch((const char *)(uintptr_t)p, _MM_HINT_T1);
 #else
-	asm volatile ("prefetcht1 %[p]" : : [p] "m" (*(const volatile char *)p));
+	/* 0 indicates intention to read, 2 sets target cache level to L2. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch((const void *)(uintptr_t)p, 0, 2);
 #endif
 }
 
@@ -40,7 +48,11 @@ static inline void rte_prefetch2(const volatile void *p)
 #ifdef RTE_TOOLCHAIN_MSVC
 	_mm_prefetch((const char *)(uintptr_t)p, _MM_HINT_T2);
 #else
-	asm volatile ("prefetcht2 %[p]" : : [p] "m" (*(const volatile char *)p));
+	/* 0 indicates intention to read, 1 sets target cache level to L3. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch((const void *)(uintptr_t)p, 0, 1);
 #endif
 }
 
@@ -49,7 +61,11 @@ static inline void rte_prefetch_non_temporal(const volatile void *p)
 #ifdef RTE_TOOLCHAIN_MSVC
 	_mm_prefetch((const char *)(uintptr_t)p, _MM_HINT_NTA);
 #else
-	asm volatile ("prefetchnta %[p]" : : [p] "m" (*(const volatile char *)p));
+	/* 0 indicates intention to read, 1 sets target cache level to L3. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch((const void *)(uintptr_t)p, 0, 0);
 #endif
 }
 
-- 
2.43.0

Re: [RFC 1/1] eal/x86: replace inline assembly prefetch with cc builtins

[Stephen Hemminger]

On Sun, 23 Feb 2025 13:52:15 +0100
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> Implement the 32- and 64-bit x86 rte_prefetch<n>() functions and
> rte_prefetch_non_temporal() using compiler builtins, rather than
> inline assembly.
> 
> This change frees the compiler to do certain optimizations that
> otherwise wouldn't be possible.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---

I like this getting rid of asm is good.
AI review did spot some things worth addressing, I don't think benchmarking
is needed. But moving to arch neutral file is good idea.

Subject: Re: [RFC 1/1] eal/x86: replace inline assembly prefetch with cc builtins

The patch still applies cleanly to main. Codegen checked with gcc 13
at -O0 and -O2, x86_64 and -m32: locality 3/2/1/0 emits
prefetcht0/t1/t2/nta, identical to the current inline asm. Worth
resending as a non-RFC after the items below.

Warning

The comment in rte_prefetch_non_temporal() is a copy/paste leftover.
It says "1 sets target cache level to L3" but the call passes 0,
which GCC maps to prefetchnta.

The same four-line comment is repeated in every function. The
locality argument is also not a cache level; the mapping to
prefetcht0/t1/t2/nta is the x86 backend's choice. Replace with a
single comment above rte_prefetch0(), e.g.:

  /*
   * On x86, __builtin_prefetch() read hints with locality 3, 2, 1, 0
   * generate prefetcht0, prefetcht1, prefetcht2 and prefetchnta.
   */

The commit message says the change "frees the compiler to do certain
optimizations" without naming them. Say what inline asm blocks
(it is opaque to the optimizer; the pointer must be materialized and
loops containing it do not vectorize) and include either a codegen
example or benchmark numbers. Also state that the generated
instruction is unchanged.

Info

With this change the non-MSVC x86 code is the same as
lib/eal/loongarch/include/rte_prefetch.h. Consider providing the
__builtin_prefetch() versions in generic/rte_prefetch.h and letting
arch headers override only where needed (MSVC, arm, ppc). That drops
duplicate code in two arch headers.

Unlike the asm, __builtin_prefetch() emits nothing on 32-bit x86
when SSE is not enabled (e.g. -march=i686). Not a problem for DPDK
builds given the SSE4.2 baseline, but applications compiling the
headers with their own flags would silently lose the prefetch.