RFR: 8247912: Make narrowOop a scoped enum

Ioi Lam iklam at openjdk.java.net
Mon Sep 21 21:44:21 UTC 2020


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.

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

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

I think a public NarrowType is confusing (vs narrowOop). Since NarrowType is rarely used, and it's unlikely to be
changed from uint32_t, maybe we should make NarrowType private, and change the above to uint32_t n =
CompressedOops::encode_as_uint32_t((oop)o); Code that does arithmetic operations on the integer value probably wants to
know the exact type anyway. I.e., not knowing whether NarrowType is signed or not makes me worry about the correctness
of `n >> 16` below.

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

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


More information about the shenandoah-dev mailing list