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

[Niclas Adlertz]

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 :)

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