RFR: 8016538: volatile double access via Unsafe.cpp is not atomic

[David Holmes]

Sorry meant to send this yesterday ...

On 10/07/2013 8:44 AM, Yumin Qi wrote:
> Vladimir,
>
> On 7/9/2013 2:16 PM, Vladimir Kozlov wrote:
>> Hi Yumin,
>>
>> load_acquire() could be more expensive with this changes even for
>> 64bit VM because you first loading from volatile address into local
>> variable. Please, check what code is generated in 64-bit with your
>> changes.
>>
> This is for 64 bits:
>
> without changeset:
>     0x00007ffff7890d67 <+167>:   movsd  -0x48(%rbp),%xmm0
>     0x00007ffff7890d6c <+172>:   movsd  %xmm0,-0x58(%rbp)
>
> with changeset:
>     0x00007ffff785ac2b <+75>:    movsd  -0x48(%rbp),%xmm0
>     0x00007ffff785ac30 <+80>:    movsd  %xmm0,-0x58(%rbp)
>
> They are same.
> Anyway, I will use macro to guard the code to keep original form in 64
> bit.

No please do not do that! We do not need conditional compilation here. 
We don't use it for jlong and should not use it for jdouble. The whole 
point of these inline OrderAccess and Atomic methods is that we can 
express things in a semantically correct form and anything not needed on 
a given platform simply disappears.

Thanks,
David

  Will send out another version of webrev.
>
> Thanks
> Yumin
>> Why you did not use release_store_fence() in
>> OrderAccess::release_store_fence() and where is cast to jlong* ?
>>
>> Thanks,
>> Vladimir
>>
>> On 7/9/13 11:32 AM, Yumin Qi wrote:
>>> Hi,
>>>
>>>    Can i have your codereview of this change:
>>>    Unsafe_Get[SET]DoubleVolatile are not atomic on 32bit x86 linux
>>> caused test for
>>> org.openjdk.concurrent.torture.tests.atomicity.primitives.reflect.DoubleAtomicityTest
>>>
>>> failed.
>>>    The fix is using same treatment for jdouble as for jlong. Tests
>>> passed on failed test case, vm quick tests. It only happens on linux
>>> 32bit platform, on solaris, the inlined code be compiled into code of
>>> using fldl/fstpl which are atomic instructions.
>>>
>>>    Contributed by: dholmes
>>>
>>>    URL: http://cr.openjdk.java.net/~minqi/8016538/01
>>>
>>>    Thanks
>>>    Yumin
>