RFR (M) 8058259: compute_offset() is confusing for static fields
Kim Barrett
kim.barrett at oracle.com
Fri Jan 5 18:59:07 UTC 2018
> On Jan 5, 2018, at 8:25 AM, coleen.phillimore at oracle.com wrote:
>
> Hi Kim, Thanks for the review (and pre-review).
>
> On 1/5/18 1:51 AM, Kim Barrett wrote:
>>> On Jan 4, 2018, at 9:29 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> Summary: remove most hard-coded offsets, have compute_offset function that takes a string and creates a TempNewSymbol, have static_field_addr() not add in InstanceMirrorKlass::offset_of_static_fields, ie use offset from find_field
>>>
>>> The jvmci code uses find_field to get the offset of static fields, then used static_field_addr() and then subtracted InstanceMirrorKlass::offset_of_static fields because the function expected the hardcoded offsets. Removed hardcoded static offsets and non-static offsets where possible.
>>>
>>> This change also removes the nonproduct flag CheckAssertionStatusDirectives (default false).
>>>
>>> Tested with tier1-5 tests on Oracle platforms, and temporary code to get failures in known class layouts, and see error logging.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8058259.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8058259
>>>
>>> thanks,
>>> Coleen
>> Looks good. Just a couple minor things. I don't need another webrev
>> for any of these.
>>
>> ------------------------------------------------------------------------------
>> src/hotspot/share/classfile/javaClasses.cpp
>> 1229 void java_lang_Thread::compute_offsets() {
>>
>> Wondering why some of the field names are referred to via
>> vmSymbols::XXX_name(), while others use string constants?
>
> I left some string constants because they were used more than once, so it seemed slightly better and not worth changing for consistency. My main purpose of doing this was so when new fields are added for classes, they don't have to be added to vmSymbols.
Okay.
>>
>> ------------------------------------------------------------------------------
>> src/hotspot/share/runtime/arguments.cpp
>> 526 { "CheckAssertionStatusDirectives",JDK_Version::undefined(), JDK_Version::jdk(10), JDK_Version::jdk(11) },
>>
>> Expired version should be jdk(12) rather than jdk(11).
>>
>> Obsolete version should be jdk(11) rather than jdk(10), but that may
>> not be possible until we've updated to be building 11.
>
> Oh I forgot to change that back. I'm waiting for the version to change, then I'll change it to 11, 12.
Okay.
>
> Thanks,
> Coleen
>>
>> ------------------------------------------------------------------------------
More information about the hotspot-dev
mailing list