llvm dev - Dec 2018 - [RFC] Matrix support (take 2)

> On Dec 5, 2018, at 8:46 PM, John McCall <jmccall at apple.com> wrote:
> 
> On 5 Dec 2018, at 13:41, Adam Nemet wrote:
>> -------------------------
>> Proper padding for small matrices
>> -------------------------
>> 
>> We want the individual rows/columns (for row/column-major) to be
naturally aligned by no more than the max vector alignment.  E.g. for a 3 x 3 x
float with column major order, we should have a single element padding after
each column which is a total of 48 bytes.  For option A, since it’s a new type
we can just define this in the new ABI.
>> 
>> For option B and C, on the other hand, vector alignment and padding is
already mandated by the VectorType.  This is part of the ABI when people use
vector_size or ext_vector_type attributes on the clang side.
>> 
>> Alignment and allocation size (including padding) is the original size
of vector rounded up to the next power-of-2.  So for example a 3 x 3 x float
pad(1) or 12 x float is rounded up to 64 bytes.  This is excessive.  I also need
to support unpadded matrices that are effectively ABI-compatible with arrays.
>> 
>> Front-ends could lower to arrays instead of vectors and then cast them
to vectors specifying the proper alignment.  This would complicate front-ends
and the IR.  A more reasonable solution would be allow reducing the alignment
and in turn the padding of the vector type itself.  We could have an align
attribute on the type itself:
>> 
>> For example <12 x float align(16)> for naturally aligned columns
or <9 x float align(1)> for the unpadded case.
>> 
>> In summary, option B and C can be made to work with some IR extensions.
> 
> In general, IR type alignment is all but meaningless.  
Sure, that’s clear.  I may not have been precise above that I actually tried to
lower the alignment in other ways but failed.
> A well-behaved frontend should be setting an explicit alignment on every
declaration, definition, allocation, and access it generates (if it's ABI,
at least), and it must always lower aggregate types in a way that understands
that IR type alignment can arbitrarily differ from the actual alignment of the
frontend's original type.
I wasn’t able to lower the alignment (and alloc size) for alloca through the
align attribute.  I assumed that it was meant to only allow increasing
alignment.

I am also unclear how lowering the alignment should work for a vector data
member of a structure type.  Would I have to make the structure packed and
manually fill in the padding?  Is that desirable every time a flattened matrix
is used inside a structure?

Thanks,
Adam
>  So while I'm certainly supportive of allowing vector types to carry
additional information besides just an element type and count, I'm a little
skeptical specifically about the value of it carrying an alignment.
> 
> John.

On 6 Dec 2018, at 1:24, Adam Nemet wrote:
>> On Dec 5, 2018, at 8:46 PM, John McCall <jmccall at apple.com>
wrote:
>>
>> On 5 Dec 2018, at 13:41, Adam Nemet wrote:
>>> -------------------------
>>> Proper padding for small matrices
>>> -------------------------
>>>
>>> We want the individual rows/columns (for row/column-major) to be 
>>> naturally aligned by no more than the max vector alignment.  E.g. 
>>> for a 3 x 3 x float with column major order, we should have a
single
>>> element padding after each column which is a total of 48 bytes. 
For
>>> option A, since it’s a new type we can just define this in the new 
>>> ABI.
>>>
>>> For option B and C, on the other hand, vector alignment and padding
>>> is already mandated by the VectorType.  This is part of the ABI
when
>>> people use vector_size or ext_vector_type attributes on the clang 
>>> side.
>>>
>>> Alignment and allocation size (including padding) is the original 
>>> size of vector rounded up to the next power-of-2.  So for example a
>>> 3 x 3 x float pad(1) or 12 x float is rounded up to 64 bytes.  This
>>> is excessive.  I also need to support unpadded matrices that are 
>>> effectively ABI-compatible with arrays.
>>>
>>> Front-ends could lower to arrays instead of vectors and then cast 
>>> them to vectors specifying the proper alignment.  This would 
>>> complicate front-ends and the IR.  A more reasonable solution would
>>> be allow reducing the alignment and in turn the padding of the 
>>> vector type itself.  We could have an align attribute on the type 
>>> itself:
>>>
>>> For example <12 x float align(16)> for naturally aligned
columns or
>>> <9 x float align(1)> for the unpadded case.
>>>
>>> In summary, option B and C can be made to work with some IR 
>>> extensions.
>>
>> In general, IR type alignment is all but meaningless.
>
> Sure, that’s clear.  I may not have been precise above that I 
> actually tried to lower the alignment in other ways but failed.
>
>> A well-behaved frontend should be setting an explicit alignment on 
>> every declaration, definition, allocation, and access it generates 
>> (if it's ABI, at least), and it must always lower aggregate types
in
>> a way that understands that IR type alignment can arbitrarily differ 
>> from the actual alignment of the frontend's original type.
>
> I wasn’t able to lower the alignment (and alloc size) for alloca 
> through the align attribute.
Oh, I see, this is meant to avoid the rounding-up of the alloc size; 
somehow I missed that part of your post.  Yes, it makes sense that you 
need to be able to specify that.
> I assumed that it was meant to only allow increasing alignment.
Are you talking about the Clang attribute?  Yes, IIRC, that attribute is 
defined as not lowering alignment unless it's combined with packed.
> I am also unclear how lowering the alignment should work for a vector 
> data member of a structure type.  Would I have to make the structure 
> packed and manually fill in the padding?  Is that desirable every time 
> a flattened matrix is used inside a structure?
That's what Clang does when the IR type for a field is aligned such that 
it would end up at the wrong offset, yes.

John.

On Dec 6, 2018, at 12:57, John McCall <jmccall at apple.com> wrote:
>> I assumed that it was meant to only allow increasing alignment.
> 
> Are you talking about the Clang attribute?  Yes, IIRC, that attribute is
defined as not lowering alignment unless it's combined with packed.
The clang attribute does not lower alignment on structs (or struct members)
without packed, but it does lower alignment on typedefs.

> On Dec 6, 2018, at 9:57 AM, John McCall <jmccall at apple.com> wrote:
> 
> 
> 
> On 6 Dec 2018, at 1:24, Adam Nemet wrote:
> 
>>> On Dec 5, 2018, at 8:46 PM, John McCall <jmccall at
apple.com> wrote:
>>> 
>>> On 5 Dec 2018, at 13:41, Adam Nemet wrote:
>>>> -------------------------
>>>> Proper padding for small matrices
>>>> -------------------------
>>>> 
>>>> We want the individual rows/columns (for row/column-major) to
be naturally aligned by no more than the max vector alignment.  E.g. for a 3 x 3
x float with column major order, we should have a single element padding after
each column which is a total of 48 bytes.  For option A, since it’s a new type
we can just define this in the new ABI.
>>>> 
>>>> For option B and C, on the other hand, vector alignment and
padding is already mandated by the VectorType.  This is part of the ABI when
people use vector_size or ext_vector_type attributes on the clang side.
>>>> 
>>>> Alignment and allocation size (including padding) is the
original size of vector rounded up to the next power-of-2.  So for example a 3 x
3 x float pad(1) or 12 x float is rounded up to 64 bytes.  This is excessive.  I
also need to support unpadded matrices that are effectively ABI-compatible with
arrays.
>>>> 
>>>> Front-ends could lower to arrays instead of vectors and then
cast them to vectors specifying the proper alignment.  This would complicate
front-ends and the IR.  A more reasonable solution would be allow reducing the
alignment and in turn the padding of the vector type itself.  We could have an
align attribute on the type itself:
>>>> 
>>>> For example <12 x float align(16)> for naturally aligned
columns or <9 x float align(1)> for the unpadded case.
>>>> 
>>>> In summary, option B and C can be made to work with some IR
extensions.
>>> 
>>> In general, IR type alignment is all but meaningless.
>> 
>> Sure, that’s clear.  I may not have been precise above that I actually
tried to lower the alignment in other ways but failed.
>> 
>>> A well-behaved frontend should be setting an explicit alignment on
every declaration, definition, allocation, and access it generates (if it's
ABI, at least), and it must always lower aggregate types in a way that
understands that IR type alignment can arbitrarily differ from the actual
alignment of the frontend's original type.
>> 
>> I wasn’t able to lower the alignment (and alloc size) for alloca
through the align attribute.
> 
> Oh, I see, this is meant to avoid the rounding-up of the alloc size;
somehow I missed that part of your post.  Yes, it makes sense that you need to
be able to specify that.
> 
>> I assumed that it was meant to only allow increasing alignment.
> 
> Are you talking about the Clang attribute?  Yes, IIRC, that attribute is
defined as not lowering alignment unless it's combined with packed.
No, I meant IR.  It obviously works on loads and stores to indicate an unaligned
access but not on alloca.  It’s probably bug, I’ll look into it.
> 
>> I am also unclear how lowering the alignment should work for a vector
data member of a structure type.  Would I have to make the structure packed and
manually fill in the padding?  Is that desirable every time a flattened matrix
is used inside a structure?
> 
> That's what Clang does when the IR type for a field is aligned such
that it would end up at the wrong offset, yes.
OK, we can adopt that model for now.

Adam
> 
> John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20181206/af886d96/attachment.html>