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

[Jorn Vernee]

Also, why aren't annotations exposed at the MemoryLayout level?

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