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