RFR: 8186166: Generalize Atomic::cmpxchg with templates
Kim Barrett
kim.barrett at oracle.com
Mon Aug 14 02:41:04 UTC 2017
Please review this change to Atomic::cmpxchg, making it a function
template with a per-platform underlying implementation.
This has been broken out from 8184334: "Generalizing Atomic with
templates", as the originally proposed change there encountered
various objections. This change takes a somewhat different approach
to the same problem.
One goal is to generalize the API and make it more type-safe,
eliminating (nearly) all need for casts by callers. Another is to
allow more direct per-platform implementations, especially where there
are appropriate compiler intrinsics available.
If all goes well and this change is accepted, it is intended that the
remainder of Atomic, and later OrderAccess, will be changed in a
similar manner as one or more followups.
This change to cmpxchg deprecates cmpxchg_ptr. Followup changes will
convert existing uses of cmpxchg_ptr and eventually remove it.
Followup changes will also update uses of cmpxchg that can be
simplified because of the generalized API.
The cmpxchg function calls private Atomic::CmpxchgImpl, to select
among several cases, based on the argument types. Each case performs
some checking and possibly conversion to reach a standard form of the
arguments (or fail and report a compile time error), which are then
passed to private Atomic::PlatformCmpxchg. The PlatformCmpxchg class
is provided by each platform, and performs additional case analysis to
generate the appropriate code for the platform.
This change involves modifications to all of the existing platform
implementations of cmpxchg. I've updated all of the platforms, but
only tested those supported by Oracle. I'll need help from the
various platform maintainers to check and test my changes.
Changes to files can be categorized as follows:
New metaprogramming utilities:
integerTypes.hpp
test_integerTypes.cpp
isRegisteredEnum.hpp
test_isRegisteredEnum.cpp
Change cmpxchg to a template:
atomic.hpp
Oracle-supported platforms:
atomic_bsd_x86.hpp - 64bit only
atomic_linux_arm.hpp
atomic_linux_x86.hpp
atomic_solaris_sparc.hpp
solaris_sparc.il
atomic_solaris_x86.hpp
atomic_windows_x86
Non-Oracle platform changes:
atomic_aix_ppc.hpp
atomic_bsd_x86.hpp - 32bit
atomic_bsd_zero.hpp
atomic_linux_aarch.hpp
atomic_linux_ppc.hpp
atomic_linux_s390.hpp
atomic_linux_sparc.hpp
atomic_linux_zero.hpp
Usage changes required by API change:
os_bsd.cpp
os_solaris.cpp
aotCodeHeap.cpp
aotCodeHeap.hpp
psParallelCompact.hpp
workgroup.cpp
oopsHierarchy.hpp
os.cpp
Example updates of cmpxchg_ptr to cmpxchg:
oop.inline.hpp
bitMap.cpp
bitMap.inline.hpp
Some specific issues:
- For Solaris (both SPARC and x86), we were using helper functions
defined in corresponding .il files. Using the cmpxchg_using_stub
helper didn't work; passing a "defined in .il" function as a template
parameter didn't work, leading to linker errors. The ideal solution
is to use gcc-style inline assembly, which is now supported by Solaris
Studio. That change was made for SPARC. However, making that change
for x86 ran into related segfaults, so using the .il definition with
direct conversions for now, until that problem can be resolved.
- I've added the function template Atomic::condition_store_ptr, which
is a cmpxchg for pointer types with a NULL compare_value. That might
not be the best name for it.
- The name IntegerTypes was retained from the original RFR for
JDK-8184334. However, that name no longer fits the provided
functionality very well, and I think a new name is needed. Something
involving Bitwise, or ValueRepresentation perhaps? I'm open to
suggestions.
- Issues with casts/conversions provided by IntegerTypes.
* Conversions between integers and floating point values are
supported. It was suggested during the discussion of the RFR for
JDK-8184334 that this was not needed, as there are no uses of
atomic/ordered floating point values in Hotspot. However, I'm told
VarHandles may include atomic floating point values.
* Conversions between integers and floating point values are presently
implemented using the memcpy technique. While this technique is
correct, it is known to be quite poorly optimized by some compilers.
It may be necessary to use the "union trick" for performance reasons,
even though it is technically undefined behavior. Hotspot already
does this elsewhere (and we should merge some of the different
conversion handling).
* In the original RFR for JDK-8184334, conversions between different
integral types of the same size (along with enums) were performed
using
reinterpret_cast<ToType&>(from_value)
This was based on discussion from several months ago, referring to
C++03 3.10/15. However, re-reading that section in response to
discussion of the RFR for JDK-8184334, I now think that implementation
is not supported in many cases needed here, and indeed invokes
undefined behavior in those cases. In particular, where 3.10/15 says
"- a type that is the signed or unsigned type corresponding to the
dynamic type of the object,"
I interpret this to mean that if the dynamic type of an object is an
integral type of a given rank, the value may be accessed using an
lvalue of an integral type of the same rank and the opposite sign.
(Using the same rank and sign is already permitted, since that's the
same type.) I don't think it allows access using an lvalue of an
integral type with a different rank. So, for example, a signed int
value may be accessed via an unsigned int lvalue. But access via a
signed long lvalue is not permitted, even if int and long have the
same size.
I don't think there is a good way to perform the integral and enum
conversions we need, without invoking implementation-defined,
unspecified, or undefined behavior.
** The memcpy technique avoids such problems, but as noted above, is
known to be quite poorly optimized by some compilers.
** Using static_cast avoids any undefined behavior, but does have
cases of implementation-defined or unspecified behavior. (When
casting an unsigned value to a signed type, the result may be
implementation defined; 5.2.9/2 and 4.7/3. C++11 <type_traits> plus
the reinterpret_cast to reference seems to provide possibly multi-step
conversion paths that avoid such qualifications on all of the steps.)
(When casting an integral value to an enum type, the result may be
unspecified; 5.2.9/7. With C++11 <type_traits>, the problem doesn't
arise so long as the starting value is appropriate, e.g. we're
reversing an enum to integer conversion.)
** Again here, we might consider the "union trick".
I haven't changed the implementation from the JDK-8184334 RFR, but I'm
presently leaning toward the "union trick", despite previous strong
resistance to it. Right now it seems to me to be the best option in
practice, despite being disallowed by the standard. It's really too
bad the memcpy technique is so poorly supported by some compilers.
CR:
https://bugs.openjdk.java.net/browse/JDK-8186166
Webrev:
http://cr.openjdk.java.net/~kbarrett/8186166/hotspot.00/
Testing:
Ad hoc Hotspot nightly test.
More information about the hotspot-dev
mailing list