[9] RFR(S): 8036851: volatile double accesses are not explicitly atomic in C2

[Christian Thalinger]

Looks good.

On May 5, 2014, at 12:54 AM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:

> Hi Niclas,
> 
> thanks for the feedback.
> 
> On 05.05.2014 10:31, Niclas Adlertz wrote:
>> Minor thing:
>> bool require_atomic_access()       { return _require_atomic_access; }
>> could be defined as:
>> bool require_atomic_access() const { return _require_atomic_access; }
>> 
>> You could also change that for LoadL/StoreL while you are at it :)
> 
> Done.
> 
> New webrev: http://cr.openjdk.java.net/~anoll/8036851/webrev.02/
> 
> Thanks,
> Tobias
> 
>> 
>> Kind Regards,
>> Niclas Adlertz
>> 
>> On 05/05/2014 09:02 AM, Tobias Hartmann wrote:
>>> Hi Christian,
>>> 
>>> thanks for the review.
>>> 
>>> On 05/02/2014 09:23 PM, Christian Thalinger wrote:
>>>> *src/share/vm/opto/memnode.hpp:*
>>>> *
>>>> *
>>>> *+   #ifndef PRODUCT*
>>>> *+     virtual void dump_spec(outputStream *st) const {*
>>>> *+       LoadNode::dump_spec(st);*
>>>> *+       if (_require_atomic_access)  st->print(" Atomic!");*
>>>> *+     }*
>>>> *+   #endif*
>>>> 
>>>> Indent is wrong.
>>> 
>>> I missed that, thanks!
>>> 
>>> New webrev: http://cr.openjdk.java.net/~anoll/8036851/webrev.01/
>>> 
>>> Best regards,
>>> Tobias
>>> 
>>>> 
>>>> Otherwise this looks good.
>>>> 
>>>> On May 2, 2014, at 2:15 AM, Tobias Hartmann
>>>> <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> please review the following patch for bug 8036851.
>>>>> 
>>>>> *Problem*
>>>>> Volatile double accesses are not treated different from normal
>>>>> accesses in the C2 compiler with respect to atomicity because on x86
>>>>> and sparc double accesses were always atomic. However, this may not
>>>>> be case on other architectures. On an architecture where there are
>>>>> atomic and non-atomic double accesses, we would have to implement all
>>>>> accesses to be atomic, because the C2 compiler does not distinguish
>>>>> between the two cases.
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8036851
>>>>> 
>>>>> *Solution*
>>>>> The C2 structure is adapted to distinguish between volatile and
>>>>> non-volatile double accesses.
>>>>> Webrev: http://cr.openjdk.java.net/~anoll/8036851/webrev.00/
>>>>> <http://cr.openjdk.java.net/%7Eanoll/8036851/webrev.00/>
>>>>> 
>>>>> *Tests*
>>>>> JPRT
>>>>> 
>>>>> Thanks,
>>>>> Tobias
>>>>> 
>>>> 
>>> 
>>> 
>