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

[Maurizio Cimadamore]

On 09/05/18 22:38, Henry Jen 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.
There's more cleanup to come to this area, for sure. Right now my goal 
is to reach parity with what we have. As we have discussed (internally) 
we plan to replace the current 'Types' abstraction almost entirely with 
something based on LayoutTypes... so I'll refrain from making changes in 
this area, at least until the new pointer/LayoutType infra is in. But 
comments noted :-)

More generally, I think ABI now is doing double duty, in the sense that 
it defines the sizes of basic types, but also has the logic for argument 
classification (for calling convention). While, in a strict 
interpretation, it is not wrong for an ABI to have these two 
ingredients, I think the aim of our layout description is precisely to 
capture the details expressed in  the 'data' part of the ABI - so that 
part feels redundant, and I think that, moving forward, we should drop 
it from the ABI abstractions, and lean on layouts more.
>
> 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?
Yep.
>
> 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.
On this part I was admittedly more hand-wavy - I get why there was a 
DataModel, but it wasn't used anywhere (only constant used was 
DataModel.LP), so I dropped it. I'm certain we can bring it back if needed.

Thanks
Maurizio
> 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
>>
>>