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
