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

[David Holmes]

On 10/07/2013 2:03 PM, Yumin Qi wrote:
> This is from comments I put on jbs:
> The same test did not fail on solaris-i586, from disassembled code:
> 0xfe9605ff: Unsafe_GetDoubleVolatile+0x00c7: addl 0x00000014(%ebp),%eax
> 0xfe960602: Unsafe_GetDoubleVolatile+0x00ca: fldl (%eax)
> 0xfe960604: Unsafe_GetDoubleVolatile+0x00cc: fstpl 0xffffffe0(%ebp)
> 0xfe960607: Unsafe_GetDoubleVolatile+0x00cf: fldl 0xffffffe0(%ebp)
> 0xfe96060a: Unsafe_GetDoubleVolatile+0x00d2: fstpl 0xffffffd0(%ebp)
> 0xfe96060d: Unsafe_GetDoubleVolatile+0x00d5: movl 0x00000004(%edi),%eax
> 0xfe960610: Unsafe_GetDoubleVolatile+0x00d8: movl %eax,0xffffffd8(%ebp)
> 0xfe960613: Unsafe_GetDoubleVolatile+0x00db: movl 0x00000008(%edi),%ecx
> 0xfe960616: Unsafe_GetDoubleVolatile+0x00de: cmpl $0x00000000,(%ecx)
> 0xfe960619: Unsafe_GetDoubleVolatile+0x00e1: je
> Unsafe_GetDoubleVolatile+0x100 [ 0xfe960638, .+0x1f ]
>
> It is using fldl/fstpl for return *p;

Interesting. Thanks,

David

>
> Thanks
> Yumin
>
> On 7/9/2013 5:40 PM, David Holmes wrote:
>> Hi Yumin,
>>
>> On 10/07/2013 4: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.
>>
>> Why is this not also a problem on Solaris? What does the Solaris
>> compiler generate for
>>
>> inline jdouble  OrderAccess::load_acquire(volatile jdouble* p) {
>> return *p; }
>>
>>>    Contributed by: dholmes
>>
>> Guess that means I can't Review it :)
>>
>> David
>> -----
>>
>>>    URL: http://cr.openjdk.java.net/~minqi/8016538/01
>>>
>>>    Thanks
>>>    Yumin
>