[foreign] RFR: 8223808: initial port for AArch64

[Maurizio Cimadamore]

On 28/05/2019 10:57, Jorn Vernee wrote:
>> * UnalignedStructTest - why doesn't it work? There's an f128 there but
>> we're not actually reading/setting it - it's mostly there to force
>> alignment a certain way
>
> I noticed it's disabled on Windows as well, but as you say, this is 
> not really needed. There will however be some mismatch between the 
> Java layout and the native one, since the size of long double is not 
> consistent. A more canonical way to add manual padding is to use 
> anonymous bitfields:
>
>   struct unaligned {
>     long long: 64;
>     long long: 64;
>     short i1;
>     short i2;
>     //padding here?
>   };
>
> That, plus adding the EXPORT macro to the functions, and replacing the 
> f128 in the layout with x128 (for clarity) works for me (I can upload 
> a webrev if wanted).

I think the test is not really testing unaligned access - the name is a 
bit misleading here.

The goal here is mostly to test whether the 
CallingSequenceBuilder/ShuffleRecipe will add some amount of SKIP in the 
resulting universal recipe, due to the fact that 'i1' comes after a 
field that is 16bytes aligned.

In other words I wanted to test this path:

http://hg.openjdk.java.net/panama/dev/file/1a1b8038185b/src/java.base/share/classes/jdk/internal/foreign/abi/x64/sysv/CallingSequenceBuilderImpl.java#l346

And how it interacted with this:

http://hg.openjdk.java.net/panama/dev/file/1a1b8038185b/src/java.base/share/classes/jdk/internal/foreign/abi/ShuffleRecipe.java#l59

In a way, the f128 in there is irrelevant. In fact you can probably put 
either x128, f128, i128 or u128 - after all since the struct will be 
passed on the stack, I believe that UniversalInvoker won't copy the 
fields separately, but will just blindly copy the struct contents in the 
appropriate argument binding destinations.

Maurizio

>
> Cheers,
> Jorn
>
> [1] : http://hg.openjdk.java.net/panama/dev/rev/3a85acb09a77#l34.2
>
> Maurizio Cimadamore schreef op 2019-05-28 10:48:
>> Looks very good. Some questions/comments:
>>
>> * don't you need some extra set of constants under NativeTypes (for
>> aarch platform?)
>>
>> * UnalignedStructTest - why doesn't it work? There's an f128 there but
>> we're not actually reading/setting it - it's mostly there to force
>> alignment a certain way
>>
>> * the change to DirectSignatureShuffler is subtle, but I think it's ok
>> and shouldn't impact SysV negatively
>>
>> The rest looks good
>>
>> Maurizio
>>
>>
>> On 28/05/2019 08:34, Nick Gasson wrote:
>>> Hi all,
>>>
>>> I've put an updated webrev for the AArch64 port here:
>>>
>>> http://cr.openjdk.java.net/~ngasson/foreign/8223808/webrev.2/
>>>
>>> This contains all the previous changes plus a few fixes to handle 
>>> things
>>> like "heterogeneous float aggregates" (structs/arrays where all the
>>> members are float/double), which are handled specially in the ABI.
>>>
>>> With this patch all the tests in the jdk_foreign group pass on AArch64,
>>> except the two I disabled pending adding long double support.
>>>
>>> Please let me know if you have any feedback!
>>>
>>> Thanks,
>>> Nick