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

[Jorn Vernee]

> 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.

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