RFR: 8264954: unified handling for VectorMask object re-materialization during de-optimization [v2]

[Vladimir Ivanov]

On Fri, 9 Apr 2021 18:56:43 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Following flow describes object reconstruction for de-optimization:-
>> 1)	PhaseVector::scalarize_vbox_node() creates SafePointScalarObjectNode to captures the box type information, also it connects to node holding the boxed value.
>> 2)	During code emit phase (PhaseOutput) C2 process above information to dumps ObjectValue holding the box information and LocationValue to holding the value information into ScopeDescriptor corresponding to Safepoint PC.
>> 3)	De-optimization blobs dump the value held in registers to the stack locations using RegisterSave::save_live_registers() and a mapping b/w register and its stack location is added to RegisterMap.
>> 4)	During de-optimization, compiled frame objects are re-allocated using identity information held in ObjectValue and their fields are initialized using values held in the stack locations accessed through register-stack mappings. 
>> 
>> By inserting a VectorStoreMaskNode before stitching the mask holding node to Safepoint we make sure that value held in opmask/vector register is transferred to a byte vector. Thus rest of the flow works as it is, stack location will hold the value in the form of a byte array irrespective of the box shape.
>> 
>> tier1-tier3 regressions are clean with UseAVX=2/3.
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8264954: Review comments resolution.

Looks good. 

Minor comments follow.

src/hotspot/share/opto/vector.cpp line 270:

> 268:     bool is_mask = is_vector_mask(iklass);
> 269:     if (is_mask) {
> 270:       if (vec_value->Opcode() != Op_VectorStoreMask) {

It's better to do the check against the type of `vec_value`: `VectorStoreMask` produces a vector of booleans  and you can guard `gvn.transform(VectorStoreMaskNode::make(...))` call with `bt != T_BOOLEAN` check instead.

src/hotspot/share/prims/vectorSupport.cpp line 98:

> 96: // and safepoint node always ensures the existence of masks in a boolean array.
> 97: //
> 98: // TODO: revisit when predicate registers are fully supported.

TODO line can be removed now.

src/hotspot/share/prims/vectorSupport.cpp line 102:

> 100: void VectorSupport::init_payload_element(typeArrayOop arr, BasicType elem_bt, int index, address addr) {
> 101:   switch (elem_bt) {
> 102:     case T_BOOLEAN:

Please, use `bool_at_put()` accessor here (just for clarity):
case T_BOOLEAN: arr->bool_at_put(index, *(jboolean*)addr); break;

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

Marked as reviewed by vlivanov (Reviewer).

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