erik.osterlund at lnu.se
Fri Jan 9 10:32:14 UTC 2015
I have some proposed changes to OrderAccess, thought I’d check if they are wanted. It comes in two parts: consistency fixing, and refactoring the code to a generalization/specialization approach instead to eliminate lots of redundant platform specific code which is not platform specific. It’s possible to pick only one if both are not wanted.
2.a) OrderAccess Maintainability
Summary: Remove platform specific code which is not platform specific (most of it) and make it shared, and separate different concerns in the code.
Different concerns are now joined to a big blob: Which barrier to use (acquire, release, fence) and where to put it w.r.t. to memory accesses, how to implement those barriers, how to make atomic memory accesses, how to specialize more efficient variants are all different concerns, now flattened out to be done over and over again for each platform even though the behaviour is very general. I propose to move everything except specialization and barrier implementation to shared code to separate their concerns, i.e. how to make atomic memory accesses and where to put the barriers w.r.t. accesses is the same for everyone except the few specialized variants with inline asm. This removes most of the code for most of the individual platforms and makes it easier to reason about the different concerns in isolation without changing stuff all over the place. The idea is to split the class into 3 classes using inheritance: OrderAccessBase, OrderAccessPlatform and OrderAccess (of course). Most of the stuff goes in OrderAccessBase, but platforms with inline assembly can put their specialized variants in OrderAccessPlatform.
2.b) OrderAccess Correctness
Summary: Relying on volatile to provide acquire/release on TSO cannot generally be guaranteed, must be fixed on all TSO except x86 linux and windows.
Correctness concerns are inconsistent over different platforms and dangerous which is my second issue with OrderAccess. Specifically, for TSO architectures like SPARC and x86, OrderAccess relies on C++ volatile for acquire release semantics, which in most cases is wrong. In every compiler I know of except MSVC 2008, this can not be relied upon as the C++ standard section 1.9 is vague whether volatiles may be reordered w.r.t. non-volatiles and has been interpreted differently by compiler implementors. It only really promises reordering constraints between volatiles which is not enough for acquire release semantics. As a matter of fact, GCC explicitly states they may be arbitrarily reordered at will, and therefore we need compiler barriers to prevent this. This problem was observed on linux and fixed by Mikael, on Linux. But the exact same problem (relying on volatile to have acquire release semantics) still lurks around on the other TSO architectures, waiting for the right moment to strike. With the refactoring, correcting such an issue (which I also intend to do because I think the current implementation is flawed) can be easily done with 3 LoC per platform - the barrier selection for memory accesses, instead of for each type of memory access on nearly every TSO platform.
The refactoring will as a bonus make a later transition to C++11/14 easier when compilers are ready, also with only a few LoC. Since we are now forced to use a compiler barrier which can be a bit too restrictive for the intended semantics, a more modern compiler can instead relax this a bit without sacrificing correctness and give us the exact semantics we want.
Any idea if this is wanted, and if so both a and b or only b? IMO it’s much easier to fix the correctness issues on different platforms with that extra bit of refactoring too. ;)
More information about the hotspot-dev