Expose generic atomicops on Clang #21
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The macro GOOGLE_PROTOBUF_ARCH_PPC is not used anywhere in the protobuf
source; there is no Power-specific atomics implementation, etc.
Funnily enough, the macro __ppc__ is not actually defined on 32-bit
Power on GCC/Linux, according to the following webpage:
http://nadeausoftware.com/articles/2012/02/c_c_tip_how_detect_processor_type_using_compiler_predefined_macros#POWER
and verified on a 32-bit Debian sid 'powerpc' chroot:
(sid_powerpc-dchroot)edmonds@partch:~$ gcc -dM -E - < /dev/null | grep -c __ppc__
0
(sid_powerpc-dchroot)edmonds@partch:~$ gcc -dM -E - < /dev/null | grep -c __LP64__
0
The generic atomicops implementation is only exposed if GCC >= 4.7 is
available, but Clang, where the underlying __atomic built-ins are also
available, typically only claims to be GCC 4.2. This causes build
failures when compiling protobuf or the output of protoc's C++ code
generator on an architecture that needs the generic atomicops
implementation with Clang.
Clang has a "c_atomic" extension which can be tested for which almost
does what we want:
C11 atomic operations
Use __has_feature(c_atomic) or __has_extension(c_atomic) to
determine if support for atomic types using _Atomic is enabled.
Clang also provides a set of builtins which can be used to implement
the <stdatomic.h> operations on _Atomic types.
I'm not sure if this guarantees that the GNU atomic builtins (the ones
with the __atomic prefix) are also available, but in practice this
should guarantee that Clang is new enough.
With this change in place, Clang generates several diagnostics when
compiling the generic atomicops implementation. These appear to be bugs
in the generic atomicops implementation and are not Clang-specific.
|
I should note, this PR is built on top of my previous PR ("Remove GOOGLE_PROTOBUF_ARCH_PPC") because it touches immediately adjacent sources lines in |
|
Thanks. I'll merge this patch in once you sign Google CLA. |
|
CLA was just submitted. |
xfxyjwf
added a commit
that referenced
this pull request
Sep 18, 2014
Expose generic atomicops on Clang
|
Oops, this causes a build failure when compiling with non-Clang. I'll submit a new PR with the fix. |
TeBoring
pushed a commit
to TeBoring/protobuf
that referenced
this pull request
Jan 19, 2019
Restructure tables for C89 port and smaller size.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The generic atomicops implementation is only exposed if GCC >= 4.7 is
available, but Clang, where the underlying __atomic built-ins are also
available, typically only claims to be GCC 4.2. This causes build
failures when compiling protobuf or the output of protoc's C++ code
generator on an architecture that needs the generic atomicops
implementation with Clang.
Clang has a "c_atomic" extension which can be tested for which almost
does what we want:
I'm not sure if this guarantees that the GNU atomic builtins (the ones
with the __atomic prefix) are also available, but in practice this
should guarantee that Clang is new enough.
With this change in place, Clang generates several diagnostics when
compiling the generic atomicops implementation. These appear to be bugs
in the generic atomicops implementation and are not Clang-specific.