[lworld] RFR: 8351550: [lworld] Some bootstrap methods/lambda factories fail with -XX:+UseNullableValueFlattening [v2]

[Frederic Parain]

On Wed, 12 Mar 2025 23:38:28 GMT, John R Rose <jrose at openjdk.org> wrote:

>> Frederic Parain has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream/lworld' into 8351550
>>  - Fix handling of offsets for boxing classes
>
> src/hotspot/share/classfile/javaClasses.cpp line 3990:
> 
>> 3988: }
>> 3989: 
>> 3990: int java_lang_boxing_object::_sub32bits_value_offset;
> 
> An existing term of art in our code for `sub32bits` is "subword".
> The `_32` and `_64` are distracting; suggest simply `int` and `long`.
> That combined with `subword` will make clear enough what is happening.
> Suggestion: `s/sub32bits/subword/; s/32bits/int/; s/64bits/long/`
> 
> Also, are we sure the subwords will always be all the same, and the floats and non-floats will correspond?  This logic assumes that.  I suppose the adjusted asserts will catch any future problems.
> 
> A fuller fix would be to make a local array indexed by `T_FOO` code, as the VM has in many other places.   (The length is `T_VOID`.)  Then each offset could be stored in its own place, customized to each `T_FOO` type.  Why not move all the way there, and forget about these ad hoc classifications?
> 
> (But did the existing asserts catch this problem in the first place?  If not why not?)

I've hesitated getting rid of the classification and doing the check for each wrapper class individually, but I was wondering why it was implemented that way initially, and that I might be missing something.
But I agree it would be a much cleaner and robust fix.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1396#discussion_r2007692600