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

[Jorn Vernee]

Ok, this is also what we talked about a bit before.

Like you said in your other email; to support varargs on Windows we'd 
need to add some `MemoryLayout asVarArg(MemoryLayout)` method on 
SystemABI that does nothing for the other 2 ABIs, there are other points 
where the ABIs might diverge that are not naturally expressible as 
MemoryLayout constants as well (though we could add a Windows.Varargs 
version of all the constants as well). It seems that exposing 
annotations so that users can add ABI specific annotation to an argument 
layout is a bit cleaner.

This also brings up the discussion of needing to account for annotations 
when checking for layout equality. I think at least for checking 
FunctionDescriptor equality we should take annotations into account as 
well. Then we can e.g. distinguish a function that takes a 64 bit float 
value normally, from one that takes a 64 bit float as a vararg on 
Windows. (similarly with int vs float values perhaps).

Jorn

On 2019-07-23 15:27, Maurizio Cimadamore wrote:
> On 23/07/2019 14:20, Jorn Vernee wrote:
>> Also, why aren't annotations exposed at the MemoryLayout level?
> 
> I wanted to experiment if we could get away w/o exposing them in the
> API - but having them internally as a more stable basis for
> implementing things like Layout names.
> 
> I'd like first to get a sense about how people will be using
> annotations. Layout names are useful and referenced by the 'path' API;
> general annotations don't have that kind of visibility in the API, so
> there's a chance they are just for us/jextract.
> 
> Maurizio
> 
>> 
>> Jorn
>> 
>> On 2019-07-23 15:07, Jorn Vernee wrote:
>>> The tests pass on Windows. Overall the patch looks good
>>> 
>>> - in TestUpcall you seem to be using the SysV.C_POINTER constant
>>> instead of the magic one.
>>> 
>>> - Do we need to distinguish between POINTER and INTEGER class?
>>> Shouldn't this just be about in which register it's passed? It looks
>>> like the isPointer predicate is only used in the tests in order to
>>> construct dummy arguments, is there another way around this? (e.g.
>>> checking that the carrier is a MemoryAddress).
>>> 
>>> Some things that I can look into:
>>> 
>>> * We can mark vararg arguments with a layout annotation on Windows to
>>> be able to support vargargs on Windows as well.
>>> 
>>> * I think some of the code in Window's CallingSequenceBuilderImpl
>>> could be removed at this point (e.g. the x87 stuff), since it's not
>>> supported by the Windows C ABI any ways.
>>> 
>>> Jorn
>>> 
>>> On 2019-07-22 19:23, Maurizio Cimadamore wrote:
>>>> Hi,
>>>> the recent push for JDK-8228447, as expected, broke the foreign-abi
>>>> branch, since that branch depended on the ValueLayout::isIntegral
>>>> accessor.
>>>> 
>>>> This patch replaces that logic with the custom layout annotation 
>>>> logic
>>>> discussed in [1]:
>>>> 
>>>> http://cr.openjdk.java.net/~mcimadamore/panama/8228486/
>>>> 
>>>> This patch makes the following changes:
>>>> 
>>>> * introduces three set of annotated layout constants, one for 
>>>> supported ABI
>>>> * the annotations point directly at the ABI-specific argument 
>>>> classes
>>>> * I've consolidated ArgumentClass a bit, so that now there's a 
>>>> common,
>>>> ABI-agnostic super-interface with some general predicates
>>>> * AddressLayout has been removed (we can just look for 
>>>> argumentClass::isPointer)
>>>> * the tests have been tweaked to use the new ABI-specific constants;
>>>> since the test has to work across platforms, all ABI test use a new
>>>> interface which imports the 'right' set of constants
>>>> 
>>>> While the patch might be a bit raw (I've only tested on Linux x64), 
>>>> I
>>>> like where this is going.
>>>> 
>>>> Maurizio