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

[Tom Rodriguez]

dean.long at oracle.com wrote on 10/5/19 9:25 PM:
> 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?

Good idea.  I've deleted those along with some other dead methods and 
rearranged the code a little bit to hide more of the API so it's clearer 
what might be done to an OopMapValue.  The delta webrev is just my 
changes and the .1 webrev is the full new webrev.  I'm submitting a 
mach5 run now.

http://cr.openjdk.java.net/~never/8231586-delta/webrev/
http://cr.openjdk.java.net/~never/8231586.1/webrev/

tom

> 
>      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
>>>
>