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

[Jatin Bhateja]

On Mon, 4 Sep 2023 03:29:48 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>>> 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.

> > > > 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.
> 
> What I'm worried is whether we can remove the `is_multifield` filter and `secondary_fields_count` check in deoptimization. Since I assumed the `secondary_fields_count` is meaningful only when the multifields are vectorized now, otherwise, the normal multifields have been added into the klass's nonstatic_fields, right? If so, only check whether the field is vector is enough for me.

You assumption still holds good, secondary_fields_count is usable only if multifiled is held in a vector, but we need secondary_fields_count to estimate the vector length unless VectorSupport::klass2length is re-introduced.

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

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