On Tue, 14 Oct 2025 11:51:09 GMT, Quan Anh Mai <[email protected]> wrote:
> Hi,
>
> Currently, the class `AdapterFingerPrint` only tracks the types of the
> arguments in the calling convention. This does not work well with the
> inclusion of value classes which need to be packed and unpacked when passed
> between different tiers. This is because 2 types can have the same sequence
> of field types, yet the fields are placed at different offsets. In those
> cases, the code to pack those types cannot be the same, hence the adapter
> code cannot be shared. Currently, a hack is implemented to cover the cases of
> inherited fields. For example:
>
> value class MyAbstract {
> int x;
> }
>
> value class MyValue1 extends MyAbstract {}
>
> value class MyValue2 {
> int x;
> }
>
> In this case, since `MyAbstract` is an abstract class, we do not know the
> complete field sequence of every class that has the field `MyAbstract.x`.
> Moreover, we need to ensure that the payload of a value class is stable when
> it is flattened, and it can be accessed atomically if atomicity is required.
> As a result, `MyAbstract.x` must have the highest alignment, which is
> currently 8 because the largest atomic flat access is 8-byte. However,
> `MyValue2.x` does not need such a large alignment.
>
> The current hack put a pair of `T_METADATA`, `T_VOID` before an inherited
> field. This is fine for the case above, but it clashes with another case:
>
> value class MyValue3 {
> @NullRestricted
> Empty e;
> int x;
> }
>
> The solution I propose is to add field offsets into `AdapterFingerPrint`, as
> well as removing the current hack. Please take a look and leave your reviews,
> thanks a lot.
Thanks for working on this! The fix looks good to me.
-------------
Marked as reviewed by thartmann (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/1679#pullrequestreview-3339644844