RFR (S) 8223472: volatile long field corruption on x86_32

[Boris Ulasevich]

Thank you!

14.05.2019 0:49, David Holmes:
> Thanks. LGTM too.
>
> David
>
> On 13/05/2019 11:57 pm, Boris Ulasevich wrote:
>> Ok. Here is the updated patch with MO_RELAXED access decorator for 
>> T_DOUBLE load/store:
>> http://cr.openjdk.java.net/~bulasevich/8223472/webrev.02
>>
>> thanks,
>> Boris
>>
>> On 13.05.2019 11:34, David Holmes wrote:
>>> On 13/05/2019 6:30 pm, Boris Ulasevich wrote:
>>>> Hi Coleen,
>>>>
>>>> I added short comment, thanks! And yes, from first look it seems 
>>>> that double load/store needs to be marked MO_RELAXED as well, but 
>>>> on x86 fstp/fld is atomic, so no extra flag is actually need.
>>>
>>> It may not actually be needed because of the underlying 
>>> implementation but the requirement should still be captured with 
>>> MO_RELAXED IMHO.
>>>
>>> Cheers,
>>> David
>>>
>>>> thank you,
>>>> Boris
>>>>
>>>> On 08.05.2019 16:56, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>> I just had a look at this and it looks unsettlingly random without 
>>>>> a short comment why MO_RELAXED is needed for long and not the 
>>>>> others. Is it because on 32 bits, double is two words that have to 
>>>>> be atomically stored?  Would a volatile double have the same 
>>>>> problem? I see getfield_or_static has the MO_RELAXED and a 
>>>>> comment, so that's good. I don't need to see a new version if you 
>>>>> add a *short* comment like this.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 5/8/19 4:17 AM, Boris Ulasevich wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Please review a simple change to fix long field store atomicity 
>>>>>> for 32-bit x86 interpreter.
>>>>>>
>>>>>> http://bugs.openjdk.java.net/browse/JDK-8223472
>>>>>> http://cr.openjdk.java.net/~bulasevich/8223472/webrev.00
>>>>>>
>>>>>> thanks,
>>>>>> Boris
>>>>>