[foreign] RFR: 8223808: initial port for AArch64

[Maurizio Cimadamore]

The patch in [1] looks more minimal, but I'm curious - it is also correct?

It seems to me that some additional SKIP steps will be added to the 
shuffle recipe, given that you now have an argument whose storage index 
is 8:

int indexInClass = 0;
   64                     for (ArgumentBinding binding : bindings) {
   65                         while (indexInClass < binding.storage().getStorageIndex()) {
   66                             builder.addSkip();
   67                             indexInClass++;
   68                         }



I guess that means that, e.g. if your native function takes nothing and 
returns a big struct, we will need to make a long array that is big 
enough to contain all register values (which are unused in this case) 
and fill it with SKIP steps, just in order to use the last step?

I think that could work - but at the same time, patch (2) seem more or 
less in the spirit of what was done with X87 registers (which are only 
supported on Intel platforms), so I can also live with that one, which 
has the advantage of being cleaner.

I wouldn't bother with DirectInvoker for now; you can enable it in a 
follow up patch if you find out what the issue is, and want to fix it - 
but as you say, it's probably better to focus on linkToNative which is 
gonna be the future here.

Cheers
Maurizio


On 20/05/2019 07:47, Nick Gasson wrote:
> Hi Maurizio,
>
> On 15/05/2019 17:51, Maurizio Cimadamore wrote:
>>
>> This seems a good direction to explore.
>>
>> I believe that, in general, our assumption that platforms have 
>> similar registers classes is only partially valid. This is visible 
>> with X87 register, which doesn't really make sense on a non-intel 
>> machine.
>>
>> But adding a class as the code is now can be problematic, as there's 
>> a 1-1 correspondence between these and ShuffleRecipeClass, which is 
>> also shared for all platforms.
>>
>> How did you go about changing that class?
>>
>> I think it could be better, for now, to stick with some hacky 
>> solution like the one you proposed above, which keeps current 
>> (broken) assumption and then revisit later a way to make the code 
>> better/more direct?
>>
>
> I've made two separate patches to implement this, please have a look 
> to see which is preferable. The first patch uses the integer argument 
> class. This puts the the indirect argument at offset 8, which 
> otherwise isn't used for an integer argument. We also now need to sort 
> the bindings when generating the shuffle recipe, as Jorn pointed out.
>
> [1] 
> http://cr.openjdk.java.net/~ngasson/foreign/8223808/struct-integer-arg/
>
> The second patch adds a new storage class to represent the indirect 
> result register. But the downside is we need to change the x86 code to 
> ignore this.
>
> [2] 
> http://cr.openjdk.java.net/~ngasson/foreign/8223808/struct-new-indirect-class/
>
> I actually think [1] may be better as it doesn't touch the x86 code 
> and is a smaller diff overall?
>
> I've also done a minor cleanup of the original patch. This disables 
> the tests that use long double on AArch64 for now.
>
> http://cr.openjdk.java.net/~ngasson/foreign/8223808/webrev.1/
>
> The StructUpcall test will pass with either patch [1] or [2] above but 
> only when using the universal invoker. I'm wondering whether we should 
> disable the direct invoker fast path on AArch64 to match Windows, as 
> this will be replaced later anyway? Or is it better to try to fix it?
>
> Thanks,
> Nick