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

[coleen.phillimore at oracle.com]

This looks good to me.
Thanks,
Coleen

On 5/13/19 9:57 AM, 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
>>>>