RFR: 8247912: Make narrowOop a scoped enum [v2]

[Stefan Karlsson]

On Tue, 22 Sep 2020 13:43:54 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change to the type narrowOop from a typedef for juint to a
>> scoped enum with uint32_t as the representation type.  This provides stronger
>> type checking when using this type.
>> 
>> For the most part this was fairly straightforward, and the patch size is
>> relatively small.  The implementation of some existing CompressedOops
>> "primitives" required adjustment. An explicit conversion to narrowOop was
>> added, with casts change to use it.  There were a few places that were type
>> punning and needed explicit conversions,, mostly in platform-specific assembly
>> support.
>> 
>> There are a couple of lingering problems.
>> 
>> Relocation::pd_set_data_value in relocInfo_ppc.cpp is treating a narrowKlass
>> as a narrowOop.  I adjusted the code to accommodate the narrowOop change, but
>> this probably ought to be done differently.
>> 
>> There are a couple of `(narrowOop)` casts remaining in s390.ad.  I'm not sure
>> whether these can be safely converted to CompressedOops::narrow_oop_cast.
>> 
>> There might still be some casts from narrowOop to an integral type.  Those are
>> hard to find in our cast-happy code base.
>> 
>> Testing:
>> tier1-6 for Oracle supported platforms.
>> Build fastdebug linux-ppc64le and linux-s390x.
>
> Kim Barrett has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - improve assertion
>  - remove NarrowType

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 179:

> 177:     // Move narrow OOP
> 178:     narrowOop noop = CompressedOops::encode((oop)o);
> 179:     uint32_t n = CompressedOops::narrow_oop_value(noop);

I wonder if we should simply create an overload for oop so that this (and the other places) could be changed to:
 n = CompressedOops::narrow_oop_value((oop)o);```

src/hotspot/cpu/ppc/relocInfo_ppc.cpp line 62:

> 60:       narrowOop no = (type() == relocInfo::oop_type) ?
> 61:           CompressedOops::encode((oop)x) :
> 62:           // FIXME: Cheating! Treating narrowKlass as a narrowOop.

I think we try to avoid adding new FIXMES and TODOS to the code. Maybe create a JBS entry instead?

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

PR: https://git.openjdk.java.net/jdk/pull/273