Re: [PATCH v2] eal/x86: fix memcpy alignment mask definition

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

On Fri, Sep 04, 2026 at 11:19:12AM +0000, Morten Brørup wrote:
> An alignment mask (ALIGNMENT_MASK) is temporarily defined for internal
> purposes, but it is publicly exposed when including the header file,
> potentially colliding with existing definitions with the same name.
> Fixed this potential issue by adding the RTE_MEMCPY_ prefix to the
> definition.
> 
> Furthermore, the superfluous function declaration at the top of the file
> was removed, and its description was moved to the function definition,
> to improve search results with source code browsers.
> 
> Also, changed "!(addrs & MASK)" to "(addrs & MASK) == 0" to follow DPDK
> coding style.
> 
> None of these changes should have any practical effect.
> 
> Fixes: f5472703c0bd ("eal: optimize aligned memcpy on x86")
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v2:
> * Changed "!(addrs & MASK)" to "(addrs & MASK) == 0". (AI)

Did you miss this change in v2? I still see the "!" below.
> ---
>  lib/eal/x86/include/rte_memcpy.h | 37 +++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/eal/x86/include/rte_memcpy.h b/lib/eal/x86/include/rte_memcpy.h
> index 8ed8c55010..41fb08ab95 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,7 @@ 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
>  
>  /**
>   * Copy 128-byte blocks from one location to another,
> @@ -333,7 +318,7 @@ 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
>  
>  /**
>   * Copy 128-byte blocks from one location to another,
> @@ -444,7 +429,7 @@ 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
>  
>  /**
>   * Macro for copying unaligned block from one location to another with constant load offset,
> @@ -673,6 +658,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)
>  {
> @@ -709,13 +706,13 @@ rte_memcpy(void *__rte_restrict dst, const void *__rte_restrict src, size_t n)
>  	}
>  
>  	/* 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
>  
>  #ifdef __cplusplus
>  }
> -- 
> 2.43.0
>