Skip to content

Clang generates atomic memory ordering warnings when compiling atomicops_internals_generic_gcc.h #25

@edmonds

Description

@edmonds

Hi,

When protobuf is patched to expose the generic atomicops implementation on Clang as in PR #21, the following warnings are generated:

./google/protobuf/stubs/atomicops_internals_generic_gcc.h:86:32: warning: memory order argument to atomic operation is invalid [-Watomic-memory-ordering]
  __atomic_store_n(ptr, value, __ATOMIC_ACQUIRE);
                               ^~~~~~~~~~~~~~~~

./google/protobuf/stubs/atomicops_internals_generic_gcc.h:102:31: warning: memory order argument to atomic operation is invalid [-Watomic-memory-ordering]
  return __atomic_load_n(ptr, __ATOMIC_RELEASE);
                              ^~~~~~~~~~~~~~~~

According to the gcc documentation at https://gcc.gnu.org/onlinedocs/gcc-4.9.1/gcc/_005f_005fatomic-Builtins.html, these arguments are indeed invalid. __atomic_store_n() cannot be passed __ATOMIC_ACQUIRE and __atomic_load_n() cannot be passed __ATOMIC_RELEASE.

I asked about this on IRC and got the following response from someone knowledgeable about atomic memory ordering.

"release + load (what the function name indicates) usually isn't a very interesting combination of behaviours."

"I doubt it has many users in protobuf?"

"if you want to get rid of the warnings you can probably add a explicit __atomic_thread_fence(__ATOMIC_RELEASE) and use __ATOMIC_RELAXED in the actually intended atomic."

"Alternatively just promote them to SEQ_CST ;)"

Indeed, a git grep of the protobuf repository shows that there aren't actually any users of the Acquire_Store() and Release_Load() functions.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions