RFR: 8263125: During deoptimization vectors should reassign scalarized payload after all objects are reallocated. [v2]

[Vladimir Ivanov]

On Wed, 10 Mar 2021 23:32:14 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> I vividly remember I fixed the very same problem before in Panama.
>> 
>> More specifically, why doesn't `VectorSupport::allocate_vector_payload` handle the case?
>> 
>> Handle VectorSupport::allocate_vector_payload(InstanceKlass* ik, frame* fr, RegisterMap* reg_map, ScopeValue* payload, TRAPS) {
>>   if (payload->is_location() &&
>>       payload->as_LocationValue()->location().type() == Location::vector) {
>>     // Vector value in an aligned adjacent tuple (1, 2, 4, 8, or 16 slots).
>>     Location location = payload->as_LocationValue()->location();
>>     return allocate_vector_payload_helper(ik, fr, reg_map, location, THREAD); // safepoint
>>   } else {
>>     // Scalar-replaced boxed vector representation.
>>     StackValue* value = StackValue::create_stack_value(fr, reg_map, payload);
>>     return value->get_obj();
>>   }
>> }
>
> Here is the order of scalarized objects in this case:
>         # jdk.incubator.vector.ShortVector::rearrangeTemplate @ bci:14 (line 2117) L[0]=#ScObj0 L[1]=_ L[2]=#ScObj1 L[3]=#ScObj2 L[4]=_ L[5]=_ STK[0]=#Ptr0x00007f83a4e99850 STK[1]=#Ptr0x00007f83bd684760 STK[2]=#Ptr0x00007f83bca050f0 STK[3]=#4 STK[4]=#ScObj0 STK[5]=#ScObj1
>         # ScObj0 jdk/incubator/vector/Short64Vector={ [payload :0]=rsp + #40 }
>         # ScObj1 jdk/incubator/vector/Short64Vector$Short64Shuffle={ [payload :0]=#ScObj3 }
>         # ScObj2 jdk/incubator/vector/Short64Vector$Short64Mask={ [payload :0]=rsp + #32 }
>         # ScObj3 byte[4]={ [0]=rsp + #48 , [1]=rsp + #52 , [2]=rsp + #56 , [3]=RBP }
> `ScObj1  Short64Shuffle` vector will be processed before `ScObj3 byte[4]` which it references.
> VectorSupport::allocate_vector_payload() called only during `ScObj1  Short64Shuffle` reallocation when `ScObj3 byte[4]` is not reallocated yet. As result `value->get_obj()` return NULL. And that is the bug.
> 
> I forgot to say that the bug is triggered only with iterative EA I am working on. Without it `byte[]` array is not scalarized and everything works.
> 
> I also thought about calling modified `VectorSupport::allocate_vector_payload()` from `Deoptimization::reassign_fields()` to keep checks in one place but it would require more code changes then suggested fix (which is mostly asserts and debug print).

Thanks for the clarifications, Vladimir. 

I agree that `VectorSupport::allocate_vector_payload` is not the right place to handle the problematic case.

Some cleanup suggestions: now you can remove `StackValue::create_stack_value()`-related code from`VectorSupport::allocate_vector_payload()`, replace `ScopeValue* payload` argument with `Location location`, and turn  
`location.type() == Location::vector` check into an assert.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2924