[lworld+vector] RFR: Merge lworld

[Jatin Bhateja]

On Mon, 26 Jun 2023 06:32:23 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

> > > > > For the scalarization changes on multifields https://github.com/jatin-bhateja/valhalla/blob/84b3b1e11cdef2c870d1f430c7db2ae60f752d5f/src/hotspot/share/opto/inlinetypenode.cpp#L327, you'v assumed that multifields are used for `vector_payload` or `vector_payload_mf` classes. Although it is true currently, it will be more reasonable if we scalarize the `InlineTypeNode` without such assumptions. What about if multifields are used elsewhere?
> > > > 
> > > > 
> > > > Yes, I plan to address it after this merge, currently there is a discrepancy b/w field count returned by _ciInstanceKlass::nof_nonstatic_fields()_ vs InlineTypeNode::field_count(), this is because ci* data model directly works over oops data model which is populated by classFileParser, thus a multifield is a regular field @multifield annotation, its only during c2 parsing we scalarize it.
> > > > We will need to add a new interface to query supported vector size in ciInstance which is implemented by both the compilers, this will be used to return correct field count which matches with IR.
> > > 
> > > 
> > > Could you please take a look at the similar changes in https://github.com/openjdk/valhalla/pull/863/files#diff-b0ea998b1ae6491c87dae54a2a70644f8e957e7f3522f780883d72fb29107aea, which the compiler firstly get the `nof_nonstatic_fields()`, and then extend the fields by the secondary fields once the field is a multifield_base. Adding another interface in `ciInstanceKlass` sounds ok, but I don't think it's a good idea creating one more similar interface. It already has `nof_nonstatic_fields` and `nof_declared_nonstatic_fields` now.
> > > If the changes under `hotspot/share/opto` in #863 looks fine to you, I can rebase it once this PR is merged, or you integrate it in this PR. Then I will abandon the 863. WDYT?
> > 
> > 
> > Yes, I grepped though the code and currently there are not many occurrences of _"nof_declared_nonstatic_fields"_ and your patch #863 is addressing them, But having consistency b/w field count returned by ci* object model and C2 IR is also desirable. BTW, its only C2 which creates Vector IR currently, hence adding a new API _ciEnv::supports_vector_size(int num_elem, BasicType elem_bt)_ which is implemented by both C1 (returning false) and C2 using _Matcher::vector_size_supported_ may cleanup some code from your patch, WDYT ?
> > All the ci* query APIs which returns ciField based on offset / index have already been extended to support multifield. But I plan to revisit them after merge patch gets integrated.
> 
> Yes, I agree that adding such an interface can simply many code. However, for the multifield vectorization, it doesn't only need to check the supported vector size, but also the relative ops with the vector size. Do you plan to check these ops as well?
> 
Correct, newly introduced interface should check for Load/Store/BroadCast Operations similar to _InlineTypeNode::is_multifield_scalarized_ 
https://github.com/jatin-bhateja/valhalla/blob/merge_lworld2/src/hotspot/share/opto/inlinetypenode.cpp#L43

> To clean more, I think it's better to add the multifields to the non_static_fields at the stage of ci field parsing based on the `vector_size_supported`, and so does to the `secondary_fields_count` just like what I did when we tried to scalarize/vectorize the multifields (see: [XiaohongGong at 90c5d21#diff-68d9ded9e6db72428f930d6e3d25ad12f804cc3550f7dde862bae0c513358220R97](https://github.com/XiaohongGong/valhalla/commit/90c5d213ee3abaa17bdac1f40c2614cd1bf9698c#diff-68d9ded9e6db72428f930d6e3d25ad12f804cc3550f7dde862bae0c513358220R97)). We have to make sure the `InlineTypeNode::fields_count()` is the same with the declared nonstatic fields from `ciInstanceKlass`. So that in the compiler stage, all things are synced and consistent. But I think this is a big refactory to multifields and deserved to revisit in future.

Yes, as mentioned I will revisit ciField side of handling after merge and will add newly discussed interface so that ci* field count is in sync with C2 IR.
![image](https://github.com/openjdk/valhalla/assets/59989778/80d9792e-f9a0-4cec-a414-f60d7ba4b880)

We are bookkeeping multifield in a special ciMultifield structure currently and  field query  APIs  based on index / offset traverses this structure.  Both offset and index based traversal should perform a linearized walk over this hierarchical structure,  since all the FieldInfo objects now contains unique indices.

JDK-8292818 added a new _index field on top regular fields specified in JVM specification (name_index, signature_index, flags_index, access_index) into FieldInfo structure, handled its initialization while parsing multifield[4], injected[5] and user defined fields[6].

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

PR Comment: https://git.openjdk.org/valhalla/pull/866#issuecomment-1606837477