RFR(S) 8231586: enlarge encoding space for OopMapValue offsets

dean.long at oracle.com dean.long at oracle.com
Sun Oct 6 04:25:10 UTC 2019


You're right, my mistake.  Your change looks good.  I was actually 
looking at the set_* methods in OopMapValue, not OopMap, and incorrectly 
thought the | would preserve the previous value.  On closer inspection, 
it doesn't, and those methods don't even get called, because the ctor 
uses a different set method.  Can we remove these unused methods?

     void set_oop()              { set_value((value() & 
register_mask_in_place) | oop_value); }
     void set_narrowoop()          { set_value((value() & 
register_mask_in_place) | narrowoop_value); }
     void set_callee_saved()     { set_value((value() & 
register_mask_in_place) | callee_saved_value); }
     void set_derived_oop()      { set_value((value() & 
register_mask_in_place) | derived_oop_value); }

dl

On 10/5/19 10:41 AM, Tom Rodriguez wrote:
>
>
> dean.long at oracle.com wrote on 10/4/19 4:27 PM:
>> It's not obvious that we only set 1 bit. The set methods don't 
>> enforce that.  And this code looks like it is setting both "derived" 
>> and "oop":
>
> Each of those calls produce independent OopMapValues. 
> http://hg.openjdk.java.net/jdk/jdk/file/ff45c1bf8129/src/hotspot/share/compiler/oopMap.cpp#l137 
>  Also all tests against the type are equality tests, not bitmasks, so 
> if more than one bit was set it would fail to match anything. The 
> bitmask-ness of the values is only used for filtering the iteration.
>
> tom
>
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/ff45c1bf8129/src/hotspot/share/opto/buildOopMap.cpp#l315 
>>
>>
>> dl
>>
>> On 10/4/19 10:26 AM, Tom Rodriguez wrote:
>>> http://cr.openjdk.java.net/~never/8231586/webrev
>>> https://bugs.openjdk.java.net/browse/JDK-8231586
>>>
>>> The current OopMapValue encoding uses a bit mask for each value even 
>>> though only one bit is ever set.  Since only 16 bits are available 
>>> for encoding this limits the offsets it can express. Compilation 
>>> with a large number of stack slots can bailout because of this 
>>> limit.  This changes the encoding to use 2 bits which gives 2 bits 
>>> back to the offset.
>>>
>>> I also deleted some StressDerivedPointers machinery that's been 
>>> completely unimplemented for years (decades?).  The flag itself is 
>>> now dead but I wasn't sure if there are test references to it 
>>> somewhere. Should I delete the flag as well?
>>>
>>> mach5 testing is in progress.
>>>
>>> tom
>>



More information about the hotspot-compiler-dev mailing list