[foreign] RFR 8216485: Binder changes are needed to support paname foreign API on Windows

[Jorn Vernee]

Followup on this.

CallingSequenceBuilderImpl and StandardCall have more difference than I 
realized. the arrangeCall method for the latter just lets you pass a 
NaitveMethodType and the argument layouts are grabbed from that. But on 
Windows we also have to differentiate between varargs and non-varargs, 
so the SysV strategy doesn't work for both. I think this is still 
solvable, of course, but I'd rather save that for a later patch if 
that's okay with you. I have moved the alignment methods into an 
abstract base class called CallingSequenceBuilder. The StorageNames 
thing was only used by the CallingSequenceBuilder implementations, so 
I've moved that logic into there as well.

I've moved ArgumentClass to the abi/x64 package (Same for it's test), 
and also created a SharedConstants class with some of the x86_64 
constant values which are also used by UniversalUpcallHandler (this was 
also referencing sysv/Constants before, which I changed to package 
access to avoid that in the future).

I've replaced the UniversalBoxer with sub classes of 
UniversalNativeInvoker and UniversalUpcallHandler.

I've changed microsoft -> windows for abi/x64/microsoft and 
Microsoftx64ABI.

updated webrev: 
http://cr.openjdk.java.net/~jvernee/panama/webrevs/windows/webrev.04/

Jorn

Maurizio Cimadamore schreef op 2019-01-10 19:34:
> On 10/01/2019 17:58, Jorn Vernee wrote:
>> Hi Maurizio,
>> 
>> Thanks for the review, comments inline...
>> 
>> Maurizio Cimadamore schreef op 2019-01-10 18:15:
>>> Continuing from [1].
>>> 
>>> This is a more in depth review of the binder changes.
>>> 
>>> 
>>> Overall the changes look good, and what I see is more or less what we
>>> have discussed over the past few weeks. As a general
>>> comment/overarching theme, I think we should try, as much as 
>>> possible,
>>> to reuse code where possible, rather then duplicating it.
>> 
>> Ok, I thought we would be going the other direction, so I did not 
>> spend much time yet on trying to de-duplicate code. I tried to touch 
>> the SysV and shared implementation as little as possible to keep 
>> maintenance easier, with the intent of de-duplicating code in a later 
>> patch if desirable.
> 
> Yes, I understand. We can also push this as is and work on incremental
> improvements, but I'd like to unify code paths as possible, as I think
> maintaining two completely separated code-bases will be trickier in
> the long run. That said, I like that, at least for prototyping, the
> new ABI design allows for slotting in new stuff quite easily.
> 
> <snip>
> 
>> - ArgumentInfo/StorageCalculator keep only one counter for the number 
>> of registers on Windows while SysV needs 2.
>> - StorageCalculator::addBindings has a special provision for varargs:
> I might have gone too far here...
>> 
>> 
>>> * The alignment routines in CallingSequenceBuilderImpl seem identical
>>> to the ones in SysVABI, so perhaps again these could be shared
>>> somewhere.
>> 
>> Same here, maybe we should bring back AbstractSystemABI? Or otherwise 
>> create an ABI utility class?
> 
> Note that the alignment function is ultimately only used by the
> CallBuilder. So, if we go for subclassing, I'd say put this in
> CallingSequenceBuilder - and then have the ABI-dependent details in
> the Windows/SysV impls.
> 
> 
> Also, I forgot an important point: please replace "microsoft" with
> "windows", as that's what's used elsewhere (e.g. in hotspot sources).
> 
> 
> Maurizio
> 
> 
>> 
>> Jorn
>> 
>>> That's all I have for now.
>>> 
>>> Cheers
>>> Maurizio
>>> 
>>> https://mail.openjdk.java.net/pipermail/panama-dev/2019-January/003680.html