RFR: 7143664: Clean up OrderAccess implementations and usage
Erik Österlund
erik.osterlund at lnu.se
Fri Feb 13 12:22:45 UTC 2015
Hi David,
Thanks for updating the webrev!
On 12/02/15 23:51, David Holmes wrote:
>> But yeah some jfloat vs jdouble stuff in the original code is very
>> strange and inconsistent. Notably, release_store_fence on linux_x86
>> properly uses compiler barriers for release for all variants except the
>> awkward jfloat, meaning it does not have the protection it needs. Looks
>> to me like this was a human mistake due to the high redundancy of the code.
>
> You mean this:
>
> OrderAccess::release_store_fence(volatile jfloat* p, jfloat v) {
> *p = v; fence();
> }
>
> technically should be: compiler_barrier(); *p =v; fence();
>
> That's an artifact of the assumption that volatile writes are sequence
> points and hence a compiler-barrier (something we now know is not the
> case with respect to preceding non-volatile accesses).
Actually, the need for compiler barriers was already known at the time
and fixed only for linux_x86 (as this is where things were observed to
crash). Release was consistently changed to use a compiler barrier for
all overloads except this jfloat overload of release_store_fence.
Of course this does not matter now as it is fixed in this patch, just an
interesting observation, showing IMO that it is easier for things like
these to slip by when code is replicated instead of generalized. ;)
Thanks for helping me with the review! Let's see if I can attract some
PPC64 people to review:
src/os_cpu/aix_ppc/vm/orderAccess_aix_ppc.inline.hpp
src/os_cpu/linux_ppc/vm/orderAccess_linux_ppc.inline.hpp
and zero people (not the number) to review:
src/os_cpu/bsd_zero/vm/orderAccess_bsd_zero.inline.hpp
src/os_cpu/linux_zero/vm/orderAccess_linux_zero.inline.hpp
Thanks,
/Erik
More information about the hotspot-dev
mailing list