[foreign-abi] RFR 8228486: Add ABI-specific layout constants

[Nick Gasson]

Hi Maurizio,

This looks OK to me and all the jdk_foreign tests are passing with the 
v3 webrev.

Thanks,
Nick


On 24/07/2019 18:37, Maurizio Cimadamore wrote:
> Nick,
> following the discussions on Layout equality, I went and changed back
> the AbstractLayout::equals method to take annotations into account.
> 
> So, I've replaced the problematic spot in the aarch64 calling sequence
> builder (isHomogeneousFloatAggregate) with this:
> 
> for (MemoryLayout elem : groupLayout.memberLayouts()) {
>               ArgumentClassImpl argClass =
>                       (ArgumentClassImpl)Utils.getAnnotation(elem,
> ArgumentClassImpl.ABI_CLASS);
>               if (!(elem instanceof ValueLayout) ||
>                       elem.bitSize() != baseType.bitSize() ||
>                       elem.bitAlignment() != baseType.bitAlignment() ||
>                       baseArgClass != argClass) {
>                   return false;
>               }
>           }
> 
> 
> I think using equals will be problematic because equals will be affected
> by names attached to the group elements.
> 
> What do you think?
> 
> Here's an updated webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8228486_v3/
> 
> Maurizio
> 
> On 24/07/2019 09:56, Maurizio Cimadamore wrote:
>> Thanks - I'll fix and push
>>
>> Maurizio
>>
>> On 24/07/2019 07:14, Nick Gasson wrote:
>>> Hi Maurizio,
>>>
>>>> Updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~mcimadamore/panama/8228486_v2/
>>>>
>>> Needs this change NativeTestHelper.java and then all the tests pass on
>>> AArch64:
>>>
>>> --- a/test/jdk/java/foreign/NativeTestHelper.java
>>> +++ b/test/jdk/java/foreign/NativeTestHelper.java
>>> @@ -48,8 +48,8 @@ public class NativeTestHelper {
>>>        public static final ValueLayout C_INT =
>>> pick(MemoryLayouts.SysV.C_INT, MemoryLayouts.WinABI.C_INT,
>>> MemoryLayouts.AArch64ABI.C_INT);
>>>        public static final ValueLayout C_FLOAT =
>>> pick(MemoryLayouts.SysV.C_FLOAT, MemoryLayouts.WinABI.C_FLOAT,
>>> MemoryLayouts.AArch64ABI.C_FLOAT);
>>>        public static final ValueLayout C_ULONG =
>>> pick(MemoryLayouts.SysV.C_ULONG, MemoryLayouts.WinABI.C_ULONG,
>>> MemoryLayouts.AArch64ABI.C_ULONG);
>>> -    public static final ValueLayout C_DOUBLE =
>>> pick(MemoryLayouts.SysV.C_DOUBLE, MemoryLayouts.WinABI.C_DOUBLE,
>>> MemoryLayouts.AArch64ABI.C_POINTER);
>>> -    public static final ValueLayout C_POINTER =
>>> pick(MemoryLayouts.SysV.C_POINTER, MemoryLayouts.WinABI.C_POINTER,
>>> MemoryLayouts.AArch64ABI.C_DOUBLE);
>>> +    public static final ValueLayout C_DOUBLE =
>>> pick(MemoryLayouts.SysV.C_DOUBLE, MemoryLayouts.WinABI.C_DOUBLE,
>>> MemoryLayouts.AArch64ABI.C_DOUBLE);
>>> +    public static final ValueLayout C_POINTER =
>>> pick(MemoryLayouts.SysV.C_POINTER, MemoryLayouts.WinABI.C_POINTER,
>>> MemoryLayouts.AArch64ABI.C_POINTER);
>>>
>>>        private static ValueLayout pick(ValueLayout sysv, ValueLayout
>>> win, ValueLayout aarch) {
>>>            if (isX86) {
>>>
>>>
>>> Thanks,
>>> Nick