Any chance someone could take a look at PR #20 ("Remove


and PR #21 ("Expose generic atomicops on Clang")?


I'm planning on uploading a patched protobuf package to Debian in the
very near future containing my patches in PR #20 and #21, as the 'mips'
build failure described below blocks the transition of protobuf 2.6.0
into the Debian archive.  So if someone from Google could look at these
patches I'd appreciate it.

[PR #20]

The GOOGLE_PROTOBUF_ARCH_PPC macro does not appear to be used anywhere
in the protobuf code base.  There is no Power-specific atomicops
implementation.  I think this would cause a build failure on Power
systems where __ppc__ is actually defined.  Notably, __ppc__ is not
defined on either 32-bit or 64-bit Power on gcc, but *is* defined on
32/64-bit Power on clang.  By luck, on Debian we happen to build
protobuf and all of its reverse build dependencies (which are also
affected, since this occurs in the headers compiled by client code) with
gcc on the 'powerpc' (32-bit Power) and 'ppc64el' (64-bit Power,
little-endian) architectures, so we have not run into this particular
build failure corner case yet.

It appears to be safe to simply remove the __ppc__ check and the
GOOGLE_PROTOBUF_ARCH_PPC define, as it is not used anywhere in protobuf.
This makes sure the generic atomicops implementation is used on Power.
This is done in PR #20.

[PR #21]

A related issue is that the generic atomicops implementation is only
exposed on GCC >= 4.7.  However, Clang also supports the __atomic_*()
intrinsics, but does not claim to be GCC >= 4.7.  So in PR #21 I
modified the preprocessor check to also support Clang.  I have an
example of a Debian build failure of one of protobuf's reverse build
dependencies because of this bug, in the 'shogun' package:


The 'shogun' package happens to be built with Clang on the Debian 'mips'
architecture, so it needs to fall back to the generic atomicops
implementation.  Note, this is big-endian MIPS, not the little-endian
'mipsel' Debian architecture, which has a custom atomicops
implementation in protobuf.  (Confusingly, what Debian calls 'mipsel',
protobuf calls 'GOOGLE_PROTOBUF_ARCH_MIPS'.)  Compiling protobuf
generated code on 'mips' with Clang results in the familiar error

    In file included from 
    In file included from /usr/lib/../include/google/protobuf/stubs/once.h:81:
    In file included from 
    /usr/lib/../include/google/protobuf/stubs/platform_macros.h:77:2: error: 
Host architecture was not detected as supported by protobuf

[Issue #25]

However, building protobuf with Clang on a generic atomicops
architecture generates new compiler diagnostics that do not appear to
have an equivalent in gcc, so I've filed issue #25:


This looks to be a genuine bug in the original generic atomicops
implementation.  These warnings appear to currently be harmless, since
the affected functions do not appear to be used anywhere in protobuf or
protobuf generated code.


Robert Edmonds

You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to protobuf+unsubscr...@googlegroups.com.
To post to this group, send email to protobuf@googlegroups.com.
Visit this group at http://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.

Reply via email to