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

[Tobias Hartmann]

Hi Chris,

thanks for the review. Do I need a second one?

Best,
Tobias

On 06.05.2014 02:26, Christian Thalinger wrote:
> 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
>>>>>>
>>>>