RFR (M): 8184334: Generalizing Atomic with templates
Kim Barrett
kim.barrett at oracle.com
Sun Aug 6 23:32:38 UTC 2017
Here is a start at addressing the various comments. I still owe email
responses to some comments, but wanted to get this out first.
http://cr.openjdk.java.net/~kbarrett/cmpxchg_template_20170806/webrev/
So far this only deals with cmpxchg, and then only for linux-x86/64.
It builds and passes basic tests. I want to get feedback on this
before spending time on other platforms, other operations, and more
extensive testing.
The metaprogramming support layer has been substantially reduced and
simplified. IntegerTypes is still there, but is more focused, with a
very small public API. It also badly needs a new name, but I'll worry
about that later.
Floating point types are still there; they can be dropped easily
enough (and reinstated if later needed). I've kept them for now
because they are easy (which is itself somewhat interesting), and
because I don't recall whether later GC interface work needed it.
I've included a mechanism for dealing with thin wrappers over
primitive types. The driver for this is oop, which is normally just a
typedef for oopDesc*, but is a class with an oopDesc* member when
CHECK_UNHANDLED_OOPS is defined (such as during fastdebug builds). We
(Erik and I) knew we would need something there, if only for the case
of oop, but hadn't agreed on a solution. Well, I'm proposing one here.
I've included a mechanism for dealing with enums. The previously
proposed change didn't handle them, as recognizing enums requires a
lot more metaprogramming, since we don't have C++11 std::is_enum. And
as mentioned earlier, I now think we need to try harder in this area.
The approach being taken is the registration mechanism I mentioned in
earlier email. The reason for including this is to allow filtering
and dispatching involving enum types, which are *not* integral types,
even though conversions are supported.
New files for metaprogramming:
metaprogramming/integerTypes.hpp
metaprogramming/registeredEnum.hpp
Atomic::cmpxchg is still changed to be function template. And it
still has three different template parameters. But there are more
constraints on the parameters, encoded in the specializations for
CmpxchgImpl. (Note that CmpxchgImpl isn't necessary; we could
accomplish the same dispatch via SFINAEed specializations for
cmpxchg. I just prefer the class vs the (IMO) syntactically horried
syntax for SFINAE of function templates. C++11 improves that a lot.
Erik prefers SFINAE of function templates rather than introducing a
helper class template like CmpxchgImpl.)
That front-end uses private Atomic::PlatformCmpxchg<size_t>
to perform . Specializations are function objects
with a template operator() with signature T(T, T volatile*, T, order).
Only Linux x86/64 updated so far. That makes use of the existing
inline assembly, but wrapped in templates rather than functions with
fixed parameter types. Replacing the assembly code with calls to
gcc's __sync_compare_and_swap would be syntactically trivial (and
indeed builds without any problems).
I've also added bool Atomic::conditional_store_ptr(T, D volatile*),
for the idiom of storing a value if the old value is NULL. It turns
out there are about 25 occurrences of this idiom in Hotspot, so a
utility for it seems warranted. The current implementation is just a
straightforward wrapper around cmpxchg, which means it can't take
advantage of gcc's __sync_bool_compare_and_swap. That can be dealt
with later if desired.
I also had to modify a few uses of cmpxchg to get this to compile.
These are presumably the same ones that Andrew encountered.
Changed files for Atomic:
runtime/atomic.hpp
os_cpu/linux_x86/vm/atomic_linux_x86.hpp
Changed files for uses, so atomic changes compile:
aot/aotCodeHeap.cpp
aot/aotCodeHeap.hpp
gc/parallel/psParallelCompact.hpp
gc/shared/workgroup.cpp
runtime/os.cpp
[Plus one additional closed change.]
I then replaced a few uses of cmpxchg_ptr with cmpxchg, taking
advantage of the new API. This eliminated a number of casts. There
are still about 110-120 uses of cmpxchg_ptr remaining.
Changed files for cmpxchg_ptr removal:
oops/oop.inline.hpp -- demonstrates oop translation
utilities/bitMap.cpp
utilities/bitMap.inline.hpp
I'm looking for feedback on this before I try to carry it any further.
More information about the hotspot-dev
mailing list