RFR (M): 7023898: Intrinsify AtomicLongFieldUpdater.getAndIncrement() (original) (raw)

Roland Westrelin roland.westrelin at oracle.com
Mon Jun 25 08:09:23 PDT 2012

Here is a new webrev: http://cr.openjdk.java.net/~roland/7023898/webrev.01/

that takes these suggestions into account:

The C2 part looks good. I have a few small suggestions.

1. Replace push(cas) by pushnode(type, loadstore). 2. I don't understand why the (kind = cmpxchg) case doesn't push a result. Does't the original push(cas) do that? 3. In the if/else chain testing 'kind', change the bare 'else' to 'else if (kind ≡ cmpxchg)' and follow up with 'else ShouldNotReachHere()'. At least put an "assert(kind ≡ cmpxchg)". This will ease maintenance, in case somebody adds a new case to loadstorekind. 4. Naming: Consider using a more type-like name for loadstorekind, such as LoadStoreKind. Compare ReexecuteState and NodeType enums. Also, the enum names themselves could use a prefix or camel-case, e.g., LSxadd or xadd or XAdd. Since the names are private to librarycall.cpp, it doesn't matter as much as in other places, but it still improves readability of code that contains the names. (Reason: an all-lower-case simple name like xadd appears at first glance to be a local variable. Manifest constants should look different.)

Roland.

More information about the hotspot-compiler-dev mailing list