[lworld+vector] RFR: 8314628: [lworld+vector] validation regression fixes and cleanups. [v2]

[Jatin Bhateja]

On Tue, 22 Aug 2023 03:10:59 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments resolutions.
>
> src/hotspot/share/ci/ciInstanceKlass.cpp line 572:
> 
>> 570: 
>> 571:   return mfield;
>> 572: }
> 
> This whole logic is quite similar with the one in `compute_nonstatic_fields_impl` when the field is a multifield. But the code style is not the same. For example, we check the `sec_fields_count` when field is multifield. This is right to me, in case other multifield is appended to a previous multifield_base which is not its expected one, although we do not have such cases now. But this similar check is missing in `compute_nonstatic_fields_impl`, isn't it? 
> 
> Another suggestion is: can we directly use `ciMultiField` instead of `ciField` as the method argument type? Besides, may I ask why we have to new another `ciMultiField`, set the secondary_fields to it and return? Can we modify the input field instead and change the method's return type to `void`?

Yes, _compute_nonstatic_fields_impl_ is only called from [ InlineTypeNode constructor](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/inlinetypenode.hpp#L40), but while parsing field access bytecodes "withfield/getfield/putfield" both c1 and c2 compiler uses _ciBytecodeStream::get_field() API_,  since it directly operates over bytecode hence does not take into account the effect of @multifield annotation over it and earlier used to create only one ciField for base multifield instead of creating a hierarchical ciMutifield structure.

In addition, ClassFiledParser creates[ contiguous sequence of FieldInfo ](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/classfile/classFileParser.cpp#L1623) for both base and subsequent synthetic multifield, hense it's not possible to associate synthetic multifields with any other base multifield.

>>> But this similar check is missing in compute_nonstatic_fields_impl, isn't it?

We can add an assertion to check that secondary fields count always match the number of synthetic FieldInfos after base multifield.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1302421179