RFR (M): 8188813: Generalize OrderAccess to use templates

[David Holmes]

On 6/10/2017 4:48 PM, Erik Österlund wrote:
> Hi David,
> 
> On fre, 2017-10-06 at 16:01 +1000, David Holmes wrote:
>> Hi Erik,
>>
>> On 5/10/2017 11:55 PM, Erik Österlund wrote:
>>>
>>> Hi,
>>>
>>> Now that Atomic has been generalized with templates, the same
>>> should to
>>> be done to OrderAccess.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8188813
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~eosterlund/8188813/webrev.00/
>> Well I didn't see anything too scary looking. :) I assume we'll drop
>> the
>> ptr variants at some stage.
> 
> Yes, that is indeed the plan.
> 
>> One query:
>>
>> src/hotspot/share/gc/shared/cardTableModRefBS.inline.hpp
>>
>> Can you declare "volatile jbyte* byte = ..." to avoid the volatile
>> cast
>> on the orderAccess call?
> 
> Sure. Fixed.
> 
>>
>>>
>>> Testing: mach5 hs-tier3
>>>
>>> Since Atomic already has a mechanism for type checking generic
>>> arguments
>>> for Atomic::load/store, and OrderAccess also is a bunch of
>>> semantically
>>> decorated loads and stores, I decided to reuse the template wheel
>>> that
>>> was already invented (Atomic::LoadImpl and Atomic::StoreImpl).
>>> Therefore, I made OrderAccess privately inherit Atomic so that
>>> this
>>> infrastructure could be reused. A whole bunch of code has been
>>> nuked
>>> with this generalization.
>> Good!
> 
> :)
> 
>>
>>>
>>> It is worth noting that I have added PrimitiveConversion
>>> functionality
>>> for doubles and floats which translates to using the union trick
>>> for
>>> casting double to and from int64_t and float to and from int32_t
>>> when
>>> passing down doubles and ints to the API. I need the former two,
>>> because
>>> Java supports volatile double and volatile float, and therefore
>>> runtime
>>> support for that needs to be able to use floats and doubles. I
>>> also
>> I didn't quite follow that. What parts of the runtime need to operate
>> on
>> volatile float/double Java fields?
> 
> At the moment, there are multiple places that support the use of Java-
> volatile float/double.
> 
> Some examples:
> * The static interpreter supports Java-volatile getfield/putfield (cf.
> cppInterpreter_zero.cpp:588, bytecodeInterpreter.cpp:2023)

Yes this is the _implementation_ of volatile field access for 
floats/doubles. I don't count that as a "use". :)

> * unsafe supports getters and setters of Java-volatile doubles/floats
> (cf. unsafe.cpp:476).

Yes this is more of a "use" but again very specific.

> This support is not accidental. The Java language allows the use of
> volatile floats and doubles. Therefore we must support them in our
> runtime.

Not quite what I meant. :) Other than the implementation of the Java 
volatile field accesses (direct of via Unsafe or intrinsics) I was 
wondering where we might need to do this. The general runtime tends not 
to do arbitary orderAccess or atomic operations on floats/doubles.

Cheers,
David

> Thanks for the review.
> 
> /Erik
> 
>>
>>>
>>> added PrimitiveConversion functionality for the subclasses of oop
>>> (instanceOop and friends). The base class oop already supported
>>> this, so
>>> it seemed natural that the subclasses should support it too.
>> Ok.
>>
>> Thanks,
>> David
>> -----
>>
>>>
>>> Thanks,
>>> /Erik