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

[Niclas Adlertz]

Looks good to me as well.

Kind Regards,
Niclas Adlertz

On 05/06/2014 08:54 AM, 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
>>>>>>>
>>>>>
>