RFR: 8247912: Make narrowOop a scoped enum

[Ioi Lam]

On Mon, 21 Sep 2020 04:01:34 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.

This looks good overall to me.

src/hotspot/share/oops/compressedOops.hpp line 142:

> 140:     // Shift by 32 is UB if size in bits of i is 32, e.g. on 32bit platform.
> 141:     assert(((i >> 16) >> 16) == 0, "narrowOop overflow");
> 142:     return static_cast<narrowOop>(static_cast<NarrowType>(i));

Should we have an assert that further restricts the range according to max heap size?

Also, should we allow only unsigned types for T? I think using signed types here is prone to error and should be
flagged.

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

Changes requested by iklam (Reviewer).

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