RFR (M): 8184334: Generalizing Atomic with templates

[Kim Barrett]

> On Jul 19, 2017, at 12:21 PM, Andrew Haley <aph at redhat.com> wrote:
> I believe this patch to be dangerous.  We might well get away with it,
> given sufficient larding of volatile, but it's still undefined
> behaviour.  We have far too much UB in HotSpot already, and we should
> not be adding any more.
> 
> And I still believe that this patch is more complex than is needed,
> and with many (if not all) targets we could do something much simpler,
> and which does not need to cast pointer types.  It could also map
> cleanly onto C++11.

I agree with Andrew that we're violating strict aliasing rules [1].
But I disagree with Andrew's assessement of the consequences.

Yes, we're violating strict aliasing.  Existing code is already doing
that.  We've been relying on volatile larding (I love that phrasing),
opacity of assembly code, and things like -fno-strict-aliasing to get
away with that.  This change doesn't change any of that.

What this change does do is allow us to centralize and isolate where
that's happening.  Current uses of Atomic are littered with casts,
along with some in the existing Atomic implementation.  By presenting
a general, type-safe, interface, we are eliminating the need for those
casts in API uses (or the oft-used alternative of making unnatural
type choices in order to match the existing limited API).  This new
implementation also does a lot of validation to improve code
correctness; that's simply missing in the existing casts-in-usage
approach.

So I think this change makes some things better (in some ways
significantly), while not being any worse than we already are on the
strict aliasing question.

Once we have this new API in place, we can go to work on cleaning up
the uses.  Independently, we can work on the implementation, which may
eventually include using C++11 atomics, at least on some platforms.
(Where applicable, that looks like it should be pretty easy with the
new API. [2]) The limitations of the present API lead to a strong
coupling between usage and implementation that make those kinds of
changes difficult.  Providing a better interface removes that coupling
and makes it easier to work on the various parts in isolation.

We also need something like this to support the GC interface work that
is in progress.  Some mechanism for connecting higher level operations
to the low-level platform-specific implementations is needed by that
work. Putting that mechanism in the atomics layer (and a forthcoming
proposal for changes to orderaccess) makes it available to more than
just the GC interface access protocol.

I agree with Erik that C++11 atomics cannot be considered a panacea in
this domain.  As he notes, there are implementation choices that are
not exposed, and which require consistency across all uses, but not
all of our interacting uses are in C++.

There is also a problem with using C++11 atomics that it generally
requires enabling C++11 support, which makes it all too easy to
unintentionally use other C++11 features.  This can easily run into
problems because there are some seriously laggard compilers being used
by some folks.  It would be very easy do some work that
unintentionally relies on some set of new language features, test,
review, and push it, and only later find out it all needed to be
backed out and re-thought due to limitations of one of these exotic
platforms.  (See discussion on the build-dev list, subject
"C++11/C++14 support in XLC ?".)  I would like to think we can find
our way through that, but it seems unlikely to happen quickly.

[1] Erik quoted 3.10: "a type that is the signed or unsigned type
corresponding to the dynamic type of the object" and suggested
"...which it is as intptr_t is the signed type corresponding to A*."

While attractive in this situation, I see no basis in the standard for
that interpretation of "corresponding".  All relevant uses of
"corresponding" are wrto integral types only.  My interpretation
(which I think matches what Andrew says about gcc's implementation) is
that "corresponding" here is about the pairs of signed/unsigned
integral types of a given size and rank.  That is, one can read a
dynamically typed signed long as an unsigned long, or vice versa.  No
more than that.

As Andrew pointed out, in the case of "A* is stored, and subsequently
loaded as intptr_t (and casted to A*)", 5.2.10/4 doesn't apply.  This
is not a cast of A* to intptr_t.  Rather, it's a cast of A** to
intptr_t* (or in the code code in question maybe some other I* where I
is an integral type).  The relevant reference is 5.2.10/7, about
converting one pointer type to another.  There's a C++11 addition
there about reinterpret_cast of pointers being equivalent to a two
step sequence of static_cast's through void* under certain conditions;
that addition codified existing practice.  But that doesn't help with
the 3.10 dereferencing issue.

[2] A couple of months ago I built Linux x86_64 with -std=gnu++11 and
ran our nightly tests, and didn't see any surprises.  We haven't
flipped that switch, in part because of the above mentioned unintended
use of new features problem (though with only one platform we
shouldn't get to the actual push part).  Also, one ad hoc run of a
subset of our tests isn't sufficient for such a change.  However, I
don't think there's anything preventing the aarch64 port from
independently doing something like that and reimplementing Atomic
using C++11 atomics, so long as the changes to shared code were done
in an appropriate manner.  I'd review that.  I might even find some
spare time to help.