RFR (M): 8184334: Generalizing Atomic with templates

Andrew Haley aph at redhat.com
Fri Jul 28 14:15:01 UTC 2017


On 27/07/17 18:27, Erik Osterlund wrote:
> Hi Andrew,
>> On 27 Jul 2017, at 17:35, Andrew Haley <aph at redhat.com> wrote:
>> Hi,
>>> On 27/07/17 15:54, Erik Österlund wrote:
>> Well, I think that was you, not me: it's simpler for everyone if we
>> insist that all of the arguments to these functions are the same type.
>> Some of the callers will have to change, sure.
> I did indeed introduce the ability to match any types, but with
> applied sanity checks protecting users from being able to compile if
> incompatible types are passed in.
> I did try earlier on to see what would happen if we forced the types
> to be identical as you propose. That turned out to be too strict. It
> made it impossible to pass in perfectly fine literals like NULL or 0
> where it should be fine (among other things). This would force too
> many call sites to have to be changed to explicitly cast passed in
> values to types that they could perfectly safely convert to
> implicitly. Therefore I decided against it.

OK.  It looked to me like some much-needed discipline for HotSpot
programmers, but I guess that's a matter of taste.  :-)

>>> Similarly, I believe similar problems may apply when different
>>> integer types of with different sizes are passed in that would not
>>> be implicitly convertible, but sure are explicitly convertible (with
>>> possibly unintentional truncation). You mentioned the case of
>>> extending a 32 bit integer to a 64 bit integer, which indeed is
>>> fine. Unfortunately, the reverse is not fine yet possible in this
>>> API. For example, if the destination is of type volatile jint* and
>>> the passed in expected value is a jlong, then that is clearly
>>> wrong. Yet the API will accept these types and explicitly convert
>>> the jlong to jint, truncating it in the process without
>>> warning. This is the kind of thing that the casting mechanism me and
>>> Kim worked on prevents you from doing accidentally.
>> OK, I see.  But it's a heck of a lot of code just to do that.
> Perhaps. But IntegerTypes is a general reusable tool that might find
> further use later on.

Hmm.  This too is perhaps a matter of taste: I'm a great believer in
not speculatively coding.  But that's a discussion for another day.

>> Instead, we could simply insist that the types must be the same, and
>> fix the callers.  This makes everything clearer, including the point
>> where the Atomic API is called.  Mismatched types at this level are
>> either unintentional oversights or plain bugs.  I have made this
>> change, and it took a few minutes to fix all eight of the callers
>> in HotSpot which have mismatched types.
> I think I already covered why this was not done. Also, that is also
> a "heck of a lot of code" invested in being able to reduce the
> anount of code in Atomic. I think you will come to the same
> conclusion once you apply that pattern to all atomic operations as
> well as OrderAccess (coming up after we are done here as it builds
> on Atomic).

I believe not.

>>> How would you feel about a mechanism where we allow each platform to
>>> specialize on the top level with all type information available, but
>>> by default send everything in to the Atomic implementation I have
>>> proposed that canonicalizes and sanity checks all values. Then we
>>> have solved a bunch of existing problems, not introduced any new
>>> problems, yet are future proof for allowing arbitrary specialization
>>> on each platform to allow further improving the solution on
>>> platforms that have cooperative compilers that allow such
>>> improvements, on a platform by platform basis.
>>> What do you think?
>> It's a better solution, and it is much more well suited to the future,
>> when we can move over to compiler intrinsics.  That's really
>> important, IMO: it gives us an escape hatch from the past.
> Okay. I am currently on my way home, but here is an untested prototype:
> http://cr.openjdk.java.net/~eosterlund/8184334/webrev.02/

Alright, let's see where this takes us.  It's be good to see if we can
get the desirable properties of only allowing safe type conversions
but still passing all of the types down to the native layer where

>> However, this would certainly fail to compile if anyone passed a
>> double.  To handle floating-point operands I would do one of two
>> things:
>> Create explicit specializations for doubles and floats: these do
>> bitwise copies into integer values when calling the assembly-language
>> sinippets, or
>> Make all of the snippets copy their arguments bitwise into integer
>> values and back.  I believe that the best-supported way of doing that
>> is the "union trick", which is widely supported by C++ compilers, even
>> when using strict aliasing.
> That makes sense. Unfortunately though, that would still violate
> section 3.10 and remain undefined behaviour.

It certainly wouldn't with GCC, because GCC defines that as an
extension, and it is thus implementation-defined.  I suspect that is
true of other compilers used to build HotSpot.  I don't much care
about theoretical niceties, but I do care about incorrect code.  The
union trick is so widely used that its UB may not matter, but I don't
have much experience of proprietary C++ compilers so I might be wrong
about that.

But never mind, we're using -fno-strict-aliasing anyway.

> Hope we are converging!

I think so.

Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

More information about the hotspot-runtime-dev mailing list