Review request (M): 7118863: Move sizeof(klassOopDesc) into the *Klass::*_offset_in_bytes() functions

[Stefan Karlsson]

On 2011-12-07 14:25, Bertrand Delsart wrote:
> Would it be possible to change the name of the offset functions ?
>
> By using the same name, but with a different semantic, you silently 
> break all the ports for which the change has not yet been applied. 
> These ports will compile correctly but will fail at execution time 
> because they will be adding sizeof(klassOopDesc) twice. Fixing them 
> will require to double check the changeset and identity all the 
> functions for which the semantic has changed. Similar issues might 
> happens if someone backports some code which includes your changes to 
> a previous hotspot version which uses the old semantic.

Good suggestion. Thanks.

StefanK

>
> By changing the names, it will be trivial to detect at build time 
> which calls have not been yet been correctly fixed. This also 
> simplifies the review since we will be sure all calls have been dealt 
> with :-)
>
> Bertrand.
>
> On 12/ 7/11 01:52 PM, Stefan Karlsson wrote:
>> Please review this REF.
>>
>> http://cr.openjdk.java.net/~stefank/7118863/webrev/
>>
>> The main motivation for this patch is to make it easier to merge with
>> the permgen project. But, IMHO, they also help the code by not having
>> different ways to go from a klass field offset to an offset against the
>> klassOopDesc.
>>
>> Two things to note about this patch.
>> 1) There's one place in the code where we don't add
>> sizeof(klassOopDesc). I maintain this oddity and will leave the correct
>> fix for a later change.
>> src/share/vm/opto/compile.cpp
>>
>> 2) I have no way to verify the shark changes.
>> src/share/vm/shark/sharkIntrinsics.cpp
>> src/share/vm/shark/sharkTopLevelBlock.cpp
>>
>>  From the RFE:
>> The *Klass::*_offset_in_bytes() functions are used to get the offset to
>> a field in a Klass. All places where they are used, we add something
>> like sizeof(klassOopDesc) to make the offset against the klassOopDesc
>> instead.
>>
>> For example in opto/memnode.cpp:
>> if (tkls->offset() == Klass::access_flags_offset_in_bytes() +
>> (int)sizeof(oopDesc)) {
>>
>> There's a couple of variants to this:
>> instanceKlass::init_thread_offset_in_bytes() + sizeof(klassOopDesc)
>> Klass::access_flags_offset_in_bytes() + (int)sizeof(oopDesc)
>> Klass::secondary_supers_offset_in_bytes() + klassOopDesc::header_size()
>> * HeapWordSize
>> Klass::java_mirror_offset_in_bytes()
>> klassOopDesc::klass_part_offset_in_bytes()
>>
>> The proposal is to move all these size adjustments into the
>> *Klass:*_offset_in_bytes() functions.
>
>