RFR: 8186166: Generalize Atomic::cmpxchg with templates
rkennke at redhat.com
Mon Aug 14 14:28:43 UTC 2017
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?
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:
> Change cmpxchg to a template:
> Oracle-supported platforms:
> atomic_bsd_x86.hpp - 64bit only
> Non-Oracle platform changes:
> atomic_bsd_x86.hpp - 32bit
> Usage changes required by API change:
> Example updates of cmpxchg_ptr to cmpxchg:
> 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
> - 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
> 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.
> Ad hoc Hotspot nightly test.
More information about the hotspot-dev