RFR (S): 8198561: Make oop and narrowOop always have their own class type
Kim Barrett
kim.barrett at oracle.com
Mon Feb 26 23:21:46 UTC 2018
> On Feb 26, 2018, at 8:32 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>
> Hi,
>
> Making oop sometimes map to class types and sometimes to primitives comes with some unfortunate problems. Advantages of making them always have their own type include:
>
> 1) Not getting compilation errors in configuration X but not Y
> 2) Making it easier to adopt existing code to use Shenandoah equals barriers
> 3) Recognize oops and narrowOops safely in template
>
> Therefore, I would like to make both oop and narrowOop always map to a class type consistently.
>
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8198561/webrev.00/
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8198561
>
> Thanks,
> /Erik
-----
Why is narrowOop::_value public?
-----
54 narrowOop& operator=(const narrowOop o) { _value = o._value; return *this; }
55 narrowOop& operator=(const PrimitiveType value) { _value = value; return *this; }
Given we have the conversion from PrimitiveType, do we need the assignment from PrimitiveType?
That wouldn’t permit direct assignment if the conversion were made explicit (which I think it should),
but then, I’m not sure that assignment from a bare PrimitiveType is really a good idea either.
-----
45 narrowOop(const PrimitiveType value) : _value(value) {}
Should this conversion be explicit? And should it permit implicit narrowing integral conversions?
The narrowing conversions could be poisoned, though that’s a bit uglier for a constructor than for
the other operations (see below).
-----
All the narrowOop operations on PrimitiveType will permit implicit narrowing integer conversions
to the PrimitiveType. That doesn’t seem like such a good idea. The narrowing conversions could
be poisoned.
-----
src/hotspot/cpu/sparc/relocInfo_sparc.cpp
99 uint32_t np = type() == relocInfo::oop_type ? (uint32_t)oopDesc::encode_heap_oop((oop)x) : Klass::encode_klass((Klass
100 inst &= ~Assembler::hi22(-1);
101 inst |= Assembler::hi22((intptr_t)(uintptr_t)np);
(1) Some text seems to have been lost at the end of line 99. I suspect this doesn’t compile.
(2) In old code, np was of type jint, and was just cast to intptr_t. Both value clauses in the initializer
return 32bit unsigned values. If the high bit of of the value can be set, then the value passed to hi22
will differ between the old code and the new.
More information about the hotspot-dev
mailing list