Hi Kim, > If I'm reading the gcc documentation for __sync_add_and_fetch > correctly, I think the following (completely untested) should work, > and won't cause Andrew to (I think rightly) complain about > type-punning reinterpret_casts. > > Though I wonder if this is a clang bug. (See below.) > > template<size_t byte_size> > struct Atomic::PlatformAdd > : Atomic::AddAndFetch<Atomic::PlatformAdd<byte_size> > > { > template<typename I, typename D> > D add_and_fetch(I add_value, D volatile* dest, atomic_memory_order order) > const { > // Cast to work around clang pointer arithmetic bug. > return PrimitiveConversions::cast<D>(__sync_add_and_fetch(dest, > add_value)); > } > }
This fails with: /home/nicgas01/jdk/src/hotspot/share/metaprogramming/primitiveConversions.hpp:162:10: error: invalid use of incomplete type 'struct PrimitiveConversions::Cast<char*, char*, true, void>' return Cast<T, U>()(x); ^~~~~~~~~~~~ I think because all the specialisations of PrimitiveConversions are only enabled if T is an integral type and here it's char*? > > The key bit from the documentation is "Operations on pointer arguments > are performed as if the operands were of the uintptr_t type." > > The bug description says this warning only arises with clang. It > seems like clang is overdoing that as-if, and treating it literally as > uintptr_t all the way through to the result type. > I think Clang is actually complaining about the argument types mismatching here. According to the GCC docs the __sync_add_and_fetch prototype looks like: type __sync_add_and_fetch (type *ptr, type value, ...) I guess Clang infers type=char* from the first argument, and complains that the second argument is unsigned long. So Clang gives an error for the following but GCC accepts it: char *ptr; unsigned long val; __sync_add_and_fetch(&ptr, val); Another way round this is to use __atomic_add_fetch instead which both Clang and GCC accept: template<typename I, typename D> D add_and_fetch(I add_value, D volatile* dest, atomic_memory_order order) const { D res = __atomic_add_fetch(dest, add_value, __ATOMIC_RELEASE); FULL_MEM_BARRIER; return res; } > Or don't change the code at all here, and say that clang is not (currently) a > supported > compiler for this platform. Yes, that's an option too :-). But we should change the configure script to give an error on --with-toolchain-type=clang on AArch64. Thanks, Nick