[foreign] RFR: replace internal nicl types with layout API

Henry Jen henry.jen at oracle.com
Wed May 9 21:40:21 UTC 2018


Oh, +1 on the webrev. Those comments can all be discussed with follow up and should not blocking any progress.

Cheers,
Henry


> On May 9, 2018, at 2:38 PM, Henry Jen <henry.jen at oracle.com> wrote:
> 
> This is good first step toward the transition.
> 
> A few comments,
> 
> 1. Types now have Layouts based on x64 ABI, but that file is not platform specific. The size of specific ABI-dependent type is suppose to be obtained from SystemABI.definedSize().  Where we should probably replace the type code with an explicit set of enum as we are posing out the type descriptor, something like following,
> // The enumeration of types defined as C standard with implementation dependent size
> enum CType {
>    Bool,
>    Char,
>    SignedChar,
>    Short,
>    Int,
>    Long,
>    LongLong,
>    UnsignedChar,
>    UnsignedShort,
>    UnsignedInt,
>    UnsignedLong,
>    UnsignedLongLong,
>    Pointer,
>    Float,
>    Double,
>    LongDouble,
> }
> 
> /**
> * The size in bytes for a type as defined by the ABI
> */
> long sizeof(CType type);
> 
> /**
> * The alignment requirement for a type as defined by the ABI
> */
> long alignment(CType type);
> 
> Then the Types layout should get the proper size from those methods.
> 
> Any place need the size of layout should be able to just get it with bitsSize method as you proposed.
> 
> 2. For now, the SystemABI.sizeof() is still used to calculate the size of a layout. How is this related to Group.bitsSize()? I assume this is temporary as we expect layout to be explicit with alignment and size?
> 
> 3. The Host.java for macosx and aix are not correct. Not sure how does the aix version even get there. :) I saw we eliminate the data model, I am not certain if we don’t need that anymore.
> 
> Cheers,
> Henry
> 
> 
>> On May 9, 2018, at 1:34 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>> 
>> Hi,
>> this patch removes the layout classes from jdk/internal/nicl/types and rewires the binder to use the public Layout API.
>> 
>> The patch is mostly straightforward, but its size is daunting - here are the major changes involved:
>> 
>> * references to internal types have been replaced with references to the layout API, more specifically:
>> 
>> types.Array -> layout.Sequence
>> types.Pointer -> layout.Address
>> types.Function -> layout.Function
>> types.Bitfield -> layout.Value
>> types.Scalar -> layout.Scalar
>> types.Container -> layout.Group
>> 
>> * Since Function is not a layout, there's no common root type between Function and Layout, meaning that some parser routines (DescriptorParser) and some utility functions (Util) had to be rewritten to take the split into account.
>> 
>> * DescriptorParser is still based on the old description language (to be replaced), but it creates Layout instances now
>> 
>> * Since Layout::bitsSize returns sizes in bits, I had to divide everything by 8, esp. in the logic for computing calling sequence recipe
>> 
>> * All test pass, but one DescriptorTest has been ignored; the reason is that this test relies heavily on the layout description of layouts, and on the assumption that there's a 1-1 mapping between such description and the output of the toString method. Since we're in a transitional phase where we are going to move to a new layout description soon, there's no point in converting this test - just to convert it again in few weeks.
>> 
>> The webrev is here:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/panama/binder%2blayout/
>> 
>> Comments welcome.
>> 
>> Cheers
>> Maurizio
>> 
>> 
> 



More information about the panama-dev mailing list