[lworld+vector] RFR: 8314980: [lworld+vector] consider scalarization conditions during ciMultiField creation. [v2]

[Jatin Bhateja]

On Wed, 30 Aug 2023 01:24:47 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>>> > Hi @jatin-bhateja, thanks for this refactoring! Code is much cleaner to me. So does it also need to clean the special handling for multifields in c1/interpreter (e.g. `deoptimize.cpp`) ? Thanks!
>>> > Besides, the `secondary_fields_count()` which is broadcasted to the `bundle_size()` of the `ciField` is calculated from `multifield_info` (see: https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/runtime/fieldDescriptor.inline.hpp#L81), which I think may not be synced with each other. And it is used widely in C2 and deoptimization. Will it have any issues?
>> 
>> 
>> I do not think it should be a problem for de-optimization,  we have separate handling for re-assignment from vector and scalars locations.
>> 
>>> > An alternative is checking the vector size supported when calculating the `secondary_fields_count()` in `fieldDescriptor.inline.hpp`. Return the multifield count if the vector size is supported, and return `1` if not. And then, we can use this information both at the ci stage when calculating the `nonstatic_fields` in `ciInstanceKlass.cpp` and C2 compiler. WDYT?
>>> 
>> 
>> I am not in favor of making changes in oop population, unless there is a pressing need, all we need is to furnish appropriate data to compilers through ci layer.  
>> 
>>> Hi @XiaohongGong , Thanks for reporting this, I was planning to revisit this in next cleanup patch, but its good to club it with this one. Will update the PR with needed modifications.
>> 
>> I revisited the implementation and captured the flow in following diagram
>> 
>> ![image](https://github.com/openjdk/valhalla/assets/59989778/c12b7a7c-2f37-4eb3-8108-0ba61687273c)
>> 
>> As discussed earlier, ClassFileParser creates separate FieldInfo structures for each synthetic multifield as its needed for field layout computations. Interpreter directly operates over oop model structures,  currently vectors are created by c2 compile only and it accesses oop model through compiler interface (ci).
>>  
>> I think to remove any ambiguity, its best to populate valid bundle_size and set is_multifield_base flag only if target vector size is able to accommodate multifield payload.
>
>> I think to remove any ambiguity, its best to populate valid bundle_size and set is_multifield_base flag only if target vector size is able to accommodate multifield payload.
> 
> Sounds reasonable to me. Thanks!

Hi @XiaohongGong , Let me know if there are any other comments.

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

PR Comment: https://git.openjdk.org/valhalla/pull/918#issuecomment-1704559333