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

[Tobias Hartmann]

Vladimir, Niclas, thanks for the reviews.

Tobias

On 06.05.2014 09:06, Vladimir Kozlov wrote:
> Looks fine to me too.
>
> Vladimir
>
> On 5/5/14 11:54 PM, Tobias Hartmann wrote:
>> 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
>>>>>>>>
>>>>>>
>>