Re: [dpdk-dev] [PATCH v4 4/4] eal/atomic: add wrapper for c11 atomics

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2020-05-13 21:40, Honnappa Nagarahalli wrote:
> <snip>
>
>>>> Subject: Re: [PATCH v4 4/4] eal/atomic: add wrapper for c11 atomics
>>>>
>>>> On Tue, May 12, 2020 at 4:03 pm, Phil Yang <mailto:phil.yang@arm.com>
>>>> wrote:
>>>>
>>>> parameter. Signed-off-by: Phil Yang <mailto:phil.yang@arm.com>
>>>>
>>>>
>>>> What is the purpose of having rte_atomic at all?
>>>> Is this level of indirection really helping?
>>>> [HONNAPPA] (not sure why this email has html format, converted to
>>>> text
>>>> format)
>>>> I believe you meant, why not use the __atomic_xxx built-ins directly?
>>>> The only reason for now is handling of
>>>> __atomic_thread_fence(__ATOMIC_SEQ_CST) for x86. This is equivalent
>>>> to rte_smp_mb which has an optimized implementation for x86.
>>>> According to Konstantin, the compiler does not generate optimal code.
>>>> Wrapping that built-in alone is going to be confusing.
>>>>
>>>> The wrappers also allow us to have our own implementation using
>>>> inline assembly for compilers versions that do not support C11 atomic
>>>> built- ins. But, I do not know if there is a need to support those versions.
>>> If I recall correctly, someone mentioned that one (or more) of the aging
>> enterprise Linux distributions don't include a compiler with C11 atomics.
>>> I think Stephen is onto something here...
>>>
>>> It is silly to add wrappers like this, if the only purpose is to support
>> compilers and distributions that don't properly support an official C standard
>> which is nearly a decade old. The quality and quantity of the DPDK
>> documentation for these functions (including examples, discussions on Stack
>> Overflow, etc.) will be inferior to the documentation of the standard C11
>> atomics, which increases the probability of incorrect use.
>>
>>
>> What's being used in DPDK today, and what's being wrapped here, is not
>> standard C11 atomics - it's a bunch of GCC built-ins. Nothing in the __
>> namespace is in the standard. It's reserved for the implementation (e.g.
>> compiler).
> I have tried to understand what it mean by 'built-ins', but I have not got a good answer. So, does it mean that the built-in function (same symbol and API interface) may not be available in another C compiler? IMO, this is what matters for DPDK.
> Currently, the same built-in functions are available in GCC and Clang.


 From what I understand, "built-ins" is GCC terminology for 
non-standard, implementation-specific intrinsic functions, built into 
the compiler. They all reside in the __* namespace.


Since GCC is the industry standard, other compilers are likely to 
follow, including built-in functions.

>>
>>> And if some compiler generates code that is suboptimal for a user, then it
>> should be the choice of the user to either accept it or use a better compiler.
>> Using a suboptimal compiler will not only affect the user's DPDK applications,
>> but all applications developed by the user. And if he accepts it for his other
>> applications, he will also accept it for his DPDK applications.
>>> We could introduce some sort of marker or standardized comment to
>> indicate when functions only exist for backwards compatibility with ancient
>> compilers and similar, with a reference to documentation describing why. And
>> when the documented preconditions are no longer relevant, e.g. when those
>> particular enterprise Linux distributions become obsolete, these functions
>> become obsolete too, and should be removed. However, getting rid of
>> obsolete cruft will break the ABI. In other words: Added cruft will never be
>> removed again, so think twice before adding.
>>>
>>> Med venlig hilsen / kind regards
>>> - Morten Brørup
>>>
>>>
>>>