Re: [PATCH v2] eal: introduce atomics abstraction

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 2/9/2023 5:40 PM, Tyler Retzlaff wrote:
> On Thu, Feb 09, 2023 at 12:53:41PM +0000, Ferruh Yigit wrote:
>> On 2/9/2023 9:04 AM, Bruce Richardson wrote:
>>> On Wed, Feb 08, 2023 at 01:43:38PM -0800, Tyler Retzlaff wrote:
>>>> Introduce atomics abstraction that permits optional use of standard C11
>>>> atomics when meson is provided the new enable_stdatomics=true option.
>>>>
>>>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>>>> ---
>>>>  config/meson.build                     | 11 ++++
>>>>  lib/eal/arm/include/rte_atomic_32.h    |  6 ++-
>>>>  lib/eal/arm/include/rte_atomic_64.h    |  6 ++-
>>>>  lib/eal/include/generic/rte_atomic.h   | 96 +++++++++++++++++++++++++++++++++-
>>>>  lib/eal/loongarch/include/rte_atomic.h |  6 ++-
>>>>  lib/eal/ppc/include/rte_atomic.h       |  6 ++-
>>>>  lib/eal/riscv/include/rte_atomic.h     |  6 ++-
>>>>  lib/eal/x86/include/rte_atomic.h       |  8 ++-
>>>>  meson_options.txt                      |  2 +
>>>>  9 files changed, 139 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/config/meson.build b/config/meson.build
>>>> index 26f3168..25dd628 100644
>>>> --- a/config/meson.build
>>>> +++ b/config/meson.build
>>>> @@ -255,6 +255,17 @@ endif
>>>>  # add -include rte_config to cflags
>>>>  add_project_arguments('-include', 'rte_config.h', language: 'c')
>>>>  
>>>> +stdc_atomics_enabled = get_option('enable_stdatomics')
>>>> +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
>>>> +
>>>> +if stdc_atomics_enabled
>>>> +if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
>>>> +    add_project_arguments('-std=gnu11', language: 'c')
>>>
>>> Is there a reason for using gnu11 on gcc and clang, rather than limiting
>>> ourselves to proper c11 support?
>>>
>>
>> +1 to stick to c11 standard instead of relying compiler extensions
> 
> the extensions are already in use. enabling -std=c11 causes warnings
> about the extensions to start being emitted.
> 

There is '__extension__' keyword (RTE_STD_C11) which is already used in
many places, can it help to remove the warnings? If it helps we can use
it, which helps to document where compiler extensions used in the code.

> but since i feel encouraged here, i think i will interpret your request
> as let's limit the scope of enablement of extensions to where they are
> already used instead of enabling them over the whole build.
> 
> i will submit a new revision patch to get rid of -std=gnu11 here.
> 
> thanks!