Re: [RFC v3 5/5] eal: provide option to use compiler memcpy instead of RTE

["Mattias Rönnblom" <hofors@lysator.liu.se>]

On 2024-06-02 22:58, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
>> Sent: Sunday, 2 June 2024 14.39
>>
>> Provide build option to have functions in <rte_memcpy.h> delegate to
>> the standard compiler/libc memcpy(), instead of using the various
>> custom DPDK, handcrafted, per-architecture rte_memcpy()
>> implementations.
>>
>> A new meson build option 'use_cc_memcpy' is added. By default,
>> the compiler/libc memcpy() is used.
>>
>> The performance benefits of the custom DPDK rte_memcpy()
>> implementations have been diminishing with every compiler release, and
>> with current toolchains the usage of a custom memcpy() implementation
>> may even be a liability.
>>
>> An additional benefit of this change is that compilers and static
>> analysis tools have an easier time detecting incorrect usage of
>> memcpy() (e.g., buffer overruns, or overlapping source and destination
>> buffers).
>>
>> This patch makes DPDK and DPDK applications using <rte_memcpy.h> use
>> compiler/libc memcpy() by default, but leaves an option to stay on the
>> custom DPDK implementations, would that prove beneficial for certain
>> applications or architectures.
>>
>> RFC v3:
>>   o Fix missing #endif on loongarch.
>>   o PPC and RISCV now implemented, meaning all architectures are
>> supported.
>>   o Unnecessary <rte_vect.h> include is removed from <rte_memcpy.h>.
>>
>> RFC v2:
>>   * Fix bug where rte_memcpy.h was not installed on x86.
>>   * Made attempt to make Loongarch compile.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> ---
> 
> We should keep pushing DPDK forward and cleaning up old cruft along the way.
> 
> The memcpy discussion has convinced me that:
> 1. This change is a good idea, and
> 2. Mainstream compilers are sufficiently mature to do it now.
> 
> So, for the series,
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
>>   static inline void
>>   rte_mov32(uint8_t *dst, const uint8_t *src);
> 
> While at it, would it be somehow beneficial to change these from uint8_t* to void* or char* (keeping const where relevant)?
> 

Something to consider before doing such a change would be if it may 
cause any strict aliasing issue for existing users.

If we should break the API, I think we are better off removing 
rte_mov*() altogether.