RFR: 8186166: Generalize Atomic::cmpxchg with templates

[Roman Kennke]

I still don't understand why we want to allow for different types for
the new and expected values and the memory location (T, D and U). We
haven't allowed for this before, why are we doing it now?

Roman

Am 14.08.2017 um 04:41 schrieb Kim Barrett:
> 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.
>
>