RFR: 8230203: Replace markWord enums with uintptr_t constants

[Kim Barrett]

> On Aug 27, 2019, at 3:32 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> Hi all,
> 
> Please review this patch to convert the enum constants in markWord into static const unintptr_t constants.
> 
> http://cr.openjdk.java.net/~stefank/8230203/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8230203
> 
> This is a follow-up RFE from the review of JDK-8229258, where a number of unnecessary casts were pointed out.
> 
> There was also a recent bug in this area, JDK-8221725, where Thomas points out the problem of relying on the bit width of the enum values.
> 
> This patch:
> 1) changes all constants to have the same type as the markWord::_value field.
> 2) Removes unnecessary uintptr_t casts
> 3) Removes uintx casts, because they are typedefed to be uintptr_t
> 4) Adds explicit casts to (int) when values are negated.
> 5) Try to only use uintptr_t inside markWord.
> 6) Leaves the use of intptr_t for parameters and return types of the different hash functions.
> 
> Tested with tier1-3
> 
> Thanks,
> StefanK


------------------------------------------------------------------------------
src/hotspot/share/oops/markWord.hpp
 134   static const uintptr_t age_bits                 = 4;
...

and
 144   static const uintptr_t lock_shift               = 0;
...

The bit counts and shift counts are used as rhs in shift expressions
and such.  I think they should be int or uint rather than uintptr_t.

------------------------------------------------------------------------------
src/hotspot/share/oops/markWord.hpp
 334     assert((uintptr_t)age <= max_age, "age too large");

I think the cast here is unnecessary and not really helpful.

Unfortunately, the cast in the immediately following line:
 335     assert((uintptr_t)bias_epoch <= max_bias_epoch, "bias epoch too large");

probably is needed to avoid -Wsign-compare warnings.  (Though
JDK-8230118 suggests we might not be getting -Wsign-compare warnings
when one might think we ought.)

------------------------------------------------------------------------------
src/hotspot/share/oops/markWord.hpp
 409     assert((size & ~size_mask) == 0, "shouldn't overflow size field");
 410     return markWord((cms_free_prototype().value() & ~size_mask_in_place) |
 411                     ((size & size_mask) << size_shift));

I wonder why 411 is applying size_mask after the 409 assert?

------------------------------------------------------------------------------
I think there were 3 casts to int added:

src/hotspot/share/opto/macro.cpp
2229                                       (~(int)markWord::age_mask_in_place), 0);

src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
3003   addi(current_header, current_header, -(int)markWord::monitor_value); // monitor

src/hotspot/cpu/aarch64/aarch64.ad
3619     __ add(tmp, tmp, -(int)markWord::monitor_value); // monitor

Consider preceeding these with STATIC_ASSERTs that the value being
cast is <= INT_MAX.

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