[lworld+vector] RFR: 8304310: Initial compilers and runtime handling for multifield backed vectors.

Xiaohong Gong xgong at openjdk.org
Mon Mar 20 07:24:51 UTC 2023


On Fri, 17 Mar 2023 14:49:24 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> Please find below the summary of changes included with the patch:
>   
> a) _ci Model:_
>    oops model creates separate fieldDescriptor/FieldInfo for each scalar field of a multifield bundle, root field is marked as multifield_base and non -  root (synthetic) fileds carry a multifield flag. To ease vector IR construction ci only exposes base multifield and ciType holding bundle size to compilers.
> 
> b) _C1 compiler:_
>    Special handling to copy entire multifield bundle during withfield bytecode processing.
> 
> c) _C2 compiler:_
>    VectorBox becomes a derivative of InlineType IR node, changes in vector box/unbox expansions. Preventing scalarization of Vector arguments and return values.
> 
> d) _Runtime:_
>    Changes in object re-construction during deoptimization since now concrete vectors have multifield based payloads. Payload (VectorPayloadMF) is a primitive class object which gets fully flattened within its parent/holding instance i.e. a concrete vector, special handling has been added for non-flattened case where payload is allocated over heap and concrete vector payload field is assigned its reference.
> 
> e) Added type specific multifield payloads to ease vector IR creation by C2, this will ensure bundle type and C2 IR type are compatible.               
> 
> Scope of current changes is limited to Vector types only, shuffles and masks are still backed by array based payloads. This PoC patch has been validated over vector only tests. More robust validation will be done in due course.
> 
> More information can be found at following link
> [https://cr.openjdk.org/~jbhateja/vectorIntrinsics/Valhalla/lworld%2Bvector.pptx](https://cr.openjdk.org/~jbhateja/vectorIntrinsics/Valhalla/lworld%2Bvector.pptx)
> 
> Please share your feedback and suggestions.
> 
> Best Regards,
> Jatin

src/hotspot/share/opto/inlinetypenode.cpp line 437:

> 435:         } else {
> 436:           value = kit->access_load_at(base, adr, adr_type, val_type, bt, decorators);
> 437:         }

Per my understanding, method "`record_method_not_compilable()`" is used here to record the compilation cannot continued by C2, right? So why do we still need to generate a `LoadVectorNode` as the field value before the recording? Will it have any issues if this node is used after then? I'm not familiar with this method. So please feel free to correct me.

src/hotspot/share/opto/inlinetypenode.cpp line 480:

> 478: 
> 479:         Node* store = kit->gvn().transform(StoreVectorNode::make(0, kit->control(), kit->memory(adr), adr, adr_type, value, vec_len));
> 480:         kit->set_memory(store, adr_type);

>From the `load` method, we know the `vec_len` of loaded vector type may be not the bundled size (i.e. the max vector size). So should the `vec_len` here come from the real bundled size instead of the vector type of the vector node?

src/hotspot/share/opto/inlinetypenode.cpp line 518:

> 516:         // all the concrete vectors should be fully flattened.
> 517:         value = value->bottom_type()->isa_vect() ? value : kit->gvn().transform(VectorNode::scalar2vector(value, vec_len, val_type, false));
> 518:         assert(value->bottom_type()->isa_vect() && value->bottom_type()->is_vect()->length() == (uint)ft->bundle_size(), "");

Is it possible that not all the multi-field values are the `value` here? For example, if the multi-fields in the InlineTypeNode have been changed before.

src/hotspot/share/opto/inlinetypenode.cpp line 725:

> 723:   Node* alloc = AllocateNode::Ideal_allocation(oop, phase);
> 724:   bool is_larval_alloc = alloc && alloc->as_Allocate()->_larval == true;
> 725:   if (!is_larval_alloc && is_default(phase) && inline_klass()->is_initialized() &&

The similar change is also merged into the valhalla mainline. See: https://github.com/openjdk/valhalla/pull/828.

src/hotspot/share/opto/inlinetypenode.cpp line 851:

> 849:     }
> 850:     if (!gvn->type(value)->is_zero_type() &&
> 851:         !VectorNode::is_all_zeros_vector(value)) {

Better to merge `VectorNode::is_all_zeros_vector(value)` with `is_zero_type()`. But it seems we have to add the constants for vector type first. Not a block.

src/hotspot/share/opto/vectornode.cpp line 861:

> 859:     return n->in(1)->bottom_type() == TypeF::ONE;
> 860:   case Op_ReplicateD:
> 861:     return n->in(1)->bottom_type() == TypeD::ONE;

`is_all_ones_vector` here means all bits are `1` (i.e. `-1` for integer types). So `TypeF::ONE` is not the right value here?

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

PR: https://git.openjdk.org/valhalla/pull/833



More information about the valhalla-dev mailing list