RFR (S): 8193063: Enabling narrowOop values for RawAccess accesses

Kim Barrett kim.barrett at oracle.com
Mon Jan 8 19:23:59 UTC 2018


> On Dec 5, 2017, at 9:49 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi,
> 
> In order to replace oopDesc::load_heap_oop() but with stricter memory ordering properties, like MO_VOLATILE, some Access tweaks are required to allow RawAccess<>::oop_load() to return narrowOop values.
> 
> I made the necessary changes to allow narrowOop values for all RawAccess operations that have an address (not the _at variants).
> 
> While I am at it, I thought I'd clean up a few things that bug me:
> 
> * The decorator verification for memory ordering specifically did not work as I originally intended, leading to harder to decipher compiler errors deeper down when using the wrong memory ordering decorators

I kind of wish that had been dealt with separately.  Oh well.

> * An unnecessary include of oop.inline.hpp was removed from the G1 barrier set.
> 
> This change helps solving the following bug:
> https://bugs.openjdk.java.net/browse/JDK-8129440
> 
> This bug is filed under:
> https://bugs.openjdk.java.net/browse/JDK-8193063
> 
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8193063/webrev.00/
> 
> Thanks,
> /Erik

------------------------------------------------------------------------------ 
src/hotspot/share/oops/access.inline.hpp 
 525       typedef RawAccessBarrier<decorators & RAW_DECORATOR_MASK> Raw;
 582       typedef RawAccessBarrier<decorators & RAW_DECORATOR_MASK> Raw;

I don't see any use of these Raw types. They look like copy-paste as
part of splitting into two specializations.

------------------------------------------------------------------------------ 
src/hotspot/share/oops/access.inline.hpp 

Too bad we don't have C++17. I think making can_hardwire_raw constexpr
and using the new "if constexpr" syntax would have been sufficient.

------------------------------------------------------------------------------ 
src/hotspot/share/oops/access.inline.hpp

I was initially thinking I wanted names for these two expressions,
since they are repeated a bunch of times:

HasDecorator<decorators, AS_RAW>::value && CanHardwireRaw<decorators>::value
HasDecorator<decorators, AS_RAW>::value && !CanHardwireRaw<decorators>::value

But I think what I really want is the better function template SFINAE
syntax allowed by C++11, e.g.

  template<DecoratorSet decorators, typename T,
           ENABLE_IF(HasDecorator<Decorators, AS_RAW>),
           DISABLE_IF(CanHardwireRaw<decorators>)>
  inline static T store(...) ...

------------------------------------------------------------------------------ 
src/hotspot/share/oops/access.inline.hpp
 796   // Step 2: Reduce types.

This comment looks like it needs updating for the narrowOop value
support.

------------------------------------------------------------------------------ 
src/hotspot/share/oops/accessBackend.hpp
 176   AccessInternal::MustConvertCompressedOop<idecorators, T>::value,

Wrong indentation.

------------------------------------------------------------------------------




More information about the hotspot-gc-dev mailing list