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

[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
>>>>