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

[Kim Barrett]

> On Sep 29, 2020, at 8:42 AM, Stefan Karlsson <stefank at openjdk.java.net> wrote:
> 
> 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);```

Nice idea. Will be in next PR update.

> 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?

Oops, forgot about that.  I’ve changed the comment to just note the type pun, and filed
https://bugs.openjdk.java.net/browse/JDK-8253860

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