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

[Tobias Hartmann]

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