[foreign-memaccess] RFR 8223978: Add alignment support to layouts

[Maurizio Cimadamore]

On 16/05/2019 11:57, Jorn Vernee wrote:
>> I think you can still model that use case - essentially what you want
>> is to just align everything to 8. (in fact that's what pragma pack(1)
>> does).
>
> Ah, yes of course! My mistake.
>
> ---
>
> Btw, while testing this I also noticed that aligning e.g. a Group with 
> alignTo() will return a Layout. I think we should use covariant return 
> types here (and in the other Layout sub-types). Also, Address is 
> missing on override for alignTo, so this will just return a Value.

Whoops - good point.

I also added a test with your 'pragma pack(1)' example. I'll spin a new 
webrev shortly.

Thanks
Maurizio

>
> Thanks,
> Jorn
>
> [1] : https://docs.microsoft.com/en-us/cpp/cpp/align-cpp?view=vs-2019
>
> Maurizio Cimadamore schreef op 2019-05-16 12:25:
>> On 15/05/2019 21:07, Jorn Vernee wrote:
>>>> * you can align a layout using Layout::alignTo(long) - this sets the
>>>> additional constraint (provided is power of two, non-negative, and
>>>> >=8)
>>>
>>> Why the power of two constraint? Shouldn't it just be a multiple of 8?
>>>
>>> This seems to preclude some packed structs, e.g. with elements with 
>>> 3 byte alignment:
>>>
>>>     #pragma pack(1)
>>>
>>>     struct Foo {
>>>         char x;
>>>         short y; // 1 byte alignment
>>>         int z; // 3 byte alignment
>>>     }; // size = 7 bytes
>>>
>>> In the layout API this is:
>>>
>>>     Value vChar = Value.ofSignedInt(8);
>>>     Value vShort = Value.ofSignedInt(16);
>>>     Value vInt = Value.ofSignedInt(32);
>>>     Group g = Group.struct(vChar, vShort.alignTo(8), vInt.alignTo(24));
>>>
>>> But this throws an IAE because alignment of 24 is not allowed.
>>
>> I think you can still model that use case - essentially what you want
>> is to just align everything to 8. (in fact that's what pragma pack(1)
>> does).
>>
>> I don't think there's such a thing as 3-byte aligned memory access: if
>> you want to read 4 bytes on a 3-byte aligned address, that's just
>> unaligned read.
>>
>> In other words, nothing new here - we're kind of following what the
>> pragma pack allows you to do: the argument to pragma pack must also be
>> a power of 2 (1, 2, 4, 8 ...), see here [1, 2] - the only difference
>> in the Panama API is that everything has to be multiplied by 8,
>> because alignment (as sizes) are expressed in bits, to have more room
>> to add sub-byte alignment (bit-fields, etc.) later on.
>>
>> So, back to your example:
>>
>> Value vChar = Value.ofSignedInt(8);
>> Value vShort = Value.ofSignedInt(16);
>> Value vInt = Value.ofSignedInt(32);
>> Group g = Group.struct(vChar.alignTo(8), vShort.alignTo(8), 
>> vInt.alignTo(8));
>>
>> Maurizio
>>
>> [1] - 
>> https://docs.microsoft.com/en-us/cpp/preprocessor/pack?view=vs-2019
>> [2] -
>> https://gcc.gnu.org/onlinedocs/gcc-4.4.4/gcc/Structure_002dPacking-Pragmas.html 
>>
>>
>>
>>>
>>> Jorn
>>>
>>> Maurizio Cimadamore schreef op 2019-05-15 19:57:
>>>> Hi,
>>>> this patch adds support to the API to express alignment constraints on
>>>> layouts. Following the model John put forward in [1], I did the
>>>> following:
>>>>
>>>> * there's a way to compute the 'natural alignment' of a layout
>>>>
>>>> * if no alignment is specified, alignment is the natural alignment
>>>>
>>>> * you can align a layout using Layout::alignTo(long) - this sets the
>>>> additional constraint (provided is power of two, non-negative, and
>>>> >=8)
>>>>
>>>> * When we obtain a VarHandle out of a Layout, at that point we are in
>>>> the process of computing offsets; here we can catch e.g. if the offset
>>>> of a given path doesn't match the specified alignment - in this case
>>>> we fail fast, even before memory is accessed; this occurs e.g. if the
>>>> layout path offset is not a multiple of its alignment, or if the
>>>> alignment of the nested element is stricter than the one of the
>>>> enclosing element.
>>>>
>>>> (in principle we could enforce these checks on layout creation, but
>>>> given we have unresolved layouts in our radar, I think it's best to do
>>>> minimal checks on layout creation and let the check fully kick in on
>>>> path creation).
>>>>
>>>> * If VarHandle can be constructed, a dynamic check verifies that
>>>> alignment of address passed to VH matches the layout requirements
>>>>
>>>> * MemoryScope::allocate also honors the alignment requirements - this
>>>> works by up-allocating memory (to make sure a pointer with desired
>>>> alignment exists in the allocated area) and then adjusting it after
>>>> the fact. For alignments < 16bytes, nothing is done given that malloc,
>>>> at least on x64 is guaranteed to respect that.
>>>>
>>>>
>>>> I think this is powerful and yet relatively simple to understand,
>>>> overall I quite like it.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mcimadamore/panama/8223978/
>>>>
>>>> Maurizio