[foreign-memaccess+abi] RFR: 8275644: Replace VMReg in shuffling code with something more fine grained.

[Jorn Vernee]

On Tue, 9 Aug 2022 13:44:37 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This patch replaces `VMReg` with a new `VMStorage` class in the C++ code, which can be used to represent stack arguments of different sizes, as well as allowing finer grained stack addressing.
>> 
>> The old masm code that used `VMReg` for moving values is also replaced with new code that uses `VMStorage`. And `VMReg` is changed to `VMStorage` everywhere.
>> 
>> The `vmstorageBase` class was added in C++ to mirror the `jdk.internal.foreign.VMStorage`. That class now also has an additional field to encode the size of a stack argument, or a register mask, which can also be used to encode the size of a register argument. This also leads to some stubs needing to be updated on unsupported platforms.
>> 
>> Lastly, instead of encoding stack indexes as 64-bit slots as we did before, they are now encoded as byte offsets, which allows finer grained stack addressing. (this leads to a bunch of values being multiplied by 8 in the Java code as well).
>> 
>> ---
>> 
>> This change is needed to address https://bugs.openjdk.org/browse/JDK-8275584 as well as adding support for `long double` as well as more exotic ABIs that take arguments as vectors.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/VMStorage.java line 31:
> 
>> 29: public class VMStorage {
>> 30:     private final byte type;
>> 31:     private final short segmentMaskOrSize;
> 
> We have a couple of fields here which do double duty - e.g. depending on whether the storage models a stack argument or a record, we end up with different meanings. One way is to use (as you did) names like `segmentMaskOrSize`.
> 
> But perhaps a better (?) way would be to make VMStorage an abstract class/interface and then have two implementations, for stack slots and registers. Although I realize this might impact the VM code in a bad way (as I think this class is mirrored there in some way).

I did my best to make the C++ `VMStorage` class fit into 64-bits, so we can rely on a scalarized ABI on all platforms. It doesn't matter that much though.

As you say, the class is mirrored in the VM code as well (in javaClass.hpp/cpp). Splitting the Java class into 2 would mean almost double the VM code that binds the class, since we then need to bind to the 2 subclasses as well.

I would like to avoid going there. Another option would be to use 2 fields, one for segment mask and one for size. But, that doesn't seem that much better. And, it also seems nice to mirror the C++ VMStorage class which uses a union for these 2 fields (in order to stay within 64-bits of size for the class).

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

PR: https://git.openjdk.org/panama-foreign/pull/699