RFR (M): 8184334: Generalizing Atomic with templates

[Andrew Haley]

Hi,

On 03/08/17 20:27, Kim Barrett wrote:
> 
> This response is about the types for a cmpxchg template.  I'll respond
> to other comments separately.
> 
> It is certainly simpler to implement if the types for all three
> arguments are required to be the same. 
> 
> I suggested Erik do that, but he reported running into "lots" of
> compilation failures.  I guess Erik was less persistent than Andrew
> in working through them.  Given Andrew's reported small number, I've
> also done that experiment and looked at the failing cases.  I think
> (nearly?) all would be improved by making the argument types match.
> There's also one similar case for xchg.
> 
> However, there are use-cases that I think are reasonable which don't
> immediately fit that restriction.
> 
> (1) cmpxchg(v, p, NULL), to store a pointer if no pointer is already
> present.  This can be used as an alternative to DCLP. One way to
> deal with this might be an overload on std::nullptr_t and use
> nullptr, but that requires C++11.  We don't have any current uses of
> this that I could find, but it's a sufficiently interesting idiom
> that I'm relucant to forbid it.  But such idiomatic usage could be
> wrapped up in its own little package that can deal with the
> restriction.
> 
> (2) The use of literals can make getting a type match more
> difficult, especially when the pointee type doesn't have portable
> syntax (like intx and uintx).  But using properly typed named values
> solves this, and may be seen as an improvement over magic literal
> values.
> 
> (3) Passing a derived pointer as the new value when updating an
> atomic pointer seems reasonable to me.  A derived pointer compare
> value seems somewhat less so.  Similarly, new and compare values
> that have pointee types that are less cv-qualified than the
> destination also seems reasonable.

I wonder if our disagreement here is perhaps a philosophical one.  C++
overload resolution rules can be obscure and sometimes surprising,
especially when using subtypes.  A cast or an assignment to a local
variable of an exactly matching type serves as a notice to the reader
of what is intended.  In my view that is beneficial: nothing is
hidden.

Because Lock-free code is inherently tricky to write, any use of
Atomic is going to require careful and sensitive analysis.  Casts of
argument types will slow down the reader slightly, but most of the
difficulty understanding such code is essential complexity rather than
accidental due to the rules of the language.

> (4) Several of the problematic cases involve enums, which aren't
> especially well handled by the original proposal. From Erik's
> explorations I knew there were some issues with enums. Having now
> looked at some of these cmpxchg problems, I think we should try
> harder to deal well with enums. One problem is that C++11
> std::is_enum is at least hard (maybe impossible?) to portabily
> emulate in C++98, so some other mechanism would be needed to
> recognize them. Probably the simplest is a registration mechanism
> for enum types that are used for atomic-access values (e.g a type
> predicate defaulting to false, and specializations for relevant enum
> types, near their definitions.)

I don't understand why this is a problem.  Conversions between enums
and integer types are well-defined.

> (5) One of our goals is to eliminate, as much as possible, the need
> for explicit casts and conversions in uses of this API.  We think the
> existing widespread use of casts makes code difficult to understand
> and is a source of bugs.  I think the small number of call sites
> affected by requiring the cmpxchg types all be the same is in no small
> measure a result of that widespread use of casts in calling code.
> Unfortunately, Hotspot is rife with boundaries across which there are
> type mismatches (and often inconsistencies even within a single
> logical chunk of code).  We don't think that's a good thing, but it's
> the context in which we were developing this change.

I believe that eliminating type mismatches is a worthwhile goal, but
this proposal doesn't quite do that.  Instead, it hides type
mismatches by coercing arguments.

Despite that we're agreed that we will need -fno-strict-aliasing for
some time to come, I still believe that gratuitous undefined behaviour
should be eliminated when it is practical to do so.  (And new UB
shouldn't be added now!)  Visible casts between compatible integer
types may be ugly, but to my mind they are far less of a sin than
unnecessary undefined behaviour.

> I think the proposed change may be more lax than it should / could
> be, but I think there are valid reasons to not require the types for
> cmpxchg arguments to all match.

There are, but IMO this is a balance of concerns.  On the one hand, a
strict requirement for some users of atomic cmpxchg.  On the other
hand, hundreds (thousands?) of lines of hairy template code.

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