Add Support for OpenCL Alignment Decoration

[tonghaining]

No description provided.

[natgavrilenko]

This might be a problem if we have two variables that use the same type (type_id) but different alignments. Let's check tomorrow what would be a better way to deal with such cases.

[tonghaining]

As we discussed since the alignment will not affect the element type, type checking (staticTypeOf) will not be an issue.

[natgavrilenko]

I think there is still a problem, see the comment to the vector-alignment test.

[tonghaining]

I added a new step to getOpVariableInitialValue() to create initial value with valid-type.

[natgavrilenko]

Hmm, this test doesn't test anything. If we remove the alignment, the test still passes. But if we try to add initial values for the variable with alignment (e.g. below), the test will crash with a parsing exception (an aggregate type created instead of an array type).

Also, we should test non-zero indexes and non-zero values, the element at 0 index is not affected by alignment.

   %arr1 = OpConstantComposite %v3uint %uint_0 %uint_1 %uint_2
   %arr2 = OpConstantComposite %v3uint %uint_3 %uint_4 %uint_5
   %arr3 = OpConstantComposite %v3uint %uint_6 %uint_7 %uint_8
   %arr4 = OpConstantComposite %v3uint %uint_9 %uint_10 %uint_11
 %value1 = OpConstantComposite %_arr_v3uint_uint_2 %arr1 %arr2
 %value2 = OpConstantComposite %_arr_v3uint_uint_2 %arr3 %arr4

%aligned_data = OpVariable %_ptr_Workgroup__arr_v3uint_uint_2 Workgroup %value1
%normal_data = OpVariable %_ptr_Workgroup__arr_v3uint_uint_2 Workgroup %value2

[tonghaining]

Test updated, the initial array type will now be cast to aggregate type based on alignment decoration.

[natgavrilenko]

I think there is still a problem, see the comment to the vector-alignment test.

[natgavrilenko]

I suggest to split this PR into two: the alignment and the new instructions. For the new instructions, there are few small comments, but overall it looks good. And having these instructions might be also useful for other tests.

For the alignment part, the conversion from array to struct doesn't look right to me. What we need is to support non-default alignments for types, i.e. it should be possible to specify a particular alignment when creating a type (i.e. alignment should be a property of a type). And later this new explicit alignment should be used to compute element size.

[ThomasHaas]

For the alignment part, the conversion from array to struct doesn't look right to me. What we need is to support non-default alignments for types, i.e. it should be possible to specify a particular alignment when creating a type (i.e. alignment should be a property of a type). And later this new explicit alignment should be used to compute element size.

Is this common in SPIRV? In C, for example, types have no alignment property. The alignment is part of the allocation site, and therefore the same type can be allocated with different alignments (IIRC, there is not even a default alignment at all).
If SPIRV requires alignment on types directly, we need to make sure that this is compatible with C.

Also, struct types have explicit control over the layout and can therefore enforce alignment.
Maybe it is sufficient to add such explicit control also to array types rather than to every type?
If we add it it to every type, we open up questions like "can we add 4-byte aligned integers with 2-byte aligned ones? If we can, what is the resulting alignment"?

[natgavrilenko]

In C, for example, types have no alignment property.

Actually, t is possible to define a type like this in C
typedef int aligned_int attribute ((aligned (256)));

If SPIRV requires alignment on types directly, we need to make sure that this is compatible with C.

First, the LLVM parser can continue using default alignment, and other languages can have user-defined alignments when needed. Second, what could be a problem with non-default alignment, assuming it is a power of 2?

Also, struct types have explicit control over the layout and can therefore enforce alignment. Maybe it is sufficient to add such explicit control also to array types rather than to every type? If we add it it to every type, we open up questions like "can we add 4-byte aligned integers with 2-byte aligned ones? If we can, what is the resulting alignment"?

Struct is not enough. For the current use case, it might be sufficient to have alignment for pointers and arrays, but it will likely lead to a lot of fragile code like "if type instanceof ScopedPOinterType and pointer.getAlignment() != null". I would prefer to have a generic solution.

[ThomasHaas]

In C, for example, types have no alignment property.

Actually, t is possible to define a type like this in C typedef int aligned_int attribute ((aligned (256)));

Yes and no. You can write it on the C-level, but the alignment is not part of the type and instead specifies alignments on all allocation sites of that type. If you look into the LLVM IR, you won't see the alignment on the type at all.
Consequently, int and aligned_int will be the same type in the IR (namely i32).

If SPIRV requires alignment on types directly, we need to make sure that this is compatible with C.

First, the LLVM parser can continue using default alignment, and other languages can have user-defined alignments when needed. Second, what could be a problem with non-default alignment, assuming it is a power of 2?

There is no problem with non-default alignment. In fact, we can even do non-power-of-two alignment.
I'm just saying that if alignment becomes part of the type, it is has to be ignored for C/LLVM code, since the allocation site specifies alignment and not the type!

@a = global i32 0, align 4, !dbg !14     // int
@b = global i32 0, align 64, !dbg !17   // aligned_int (same type)

That being said, for struct types the alignment can yield different types if the alignment is too large and padding bits need to be inserted.

struct { long x; long y; } __attribute__((aligned(16))) s1;
struct { long x; long y; } s2; 
// yields:
%struct.anon = type { i64, i64 }
%struct.anon.0 = type { i64, i64 }
@s1 = global %struct.anon zeroinitializer, align 16, !dbg !21    // Alignment is reflected here: type is the same!
@s2 = global %struct.anon.0 zeroinitializer, align 8, !dbg !28

//----------
struct { long x; long y; } __attribute__((aligned(32))) s1; // Overaligned
struct { long x; long y; } s2; 
// yields:
%struct.anon = type { i64, i64, [16 x i8] }     // Additional padding bits due to overalignment (mainly for arrays).
%struct.anon.0 = type { i64, i64 }
@s1 = global %struct.anon zeroinitializer, align 32, !dbg !21.    // Alignment still only stated at allocation site
@s2 = global %struct.anon.0 zeroinitializer, align 8, !dbg !28

I'm wondering if SPIRV really does it differently and bakes it into the type or not. From the documentation I can see that SPIRV allows multiple identical types to be defined with possibly different decorations, but I didn't see any alignment decorations.

EDIT: Checking the benchmarks of this PR, it seems SPIRV does not attach alignments to types either. The decorations are on allocation sites as far as I can tell.
EDIT_END.

Also, struct types have explicit control over the layout and can therefore enforce alignment. Maybe it is sufficient to add such explicit control also to array types rather than to every type? If we add it it to every type, we open up questions like "can we add 4-byte aligned integers with 2-byte aligned ones? If we can, what is the resulting alignment"?

Struct is not enough. For the current use case, it might be sufficient to have alignment for pointers and arrays, but it will likely lead to a lot of fragile code like "if type instanceof ScopedPOinterType and pointer.getAlignment() != null". I would prefer to have a generic solution.

Well, when you make alignment part of the type, you need to do alignment checks for each and every expression in the IR, or weaken the type checking to disregard alignments. Still, for most operations on types alignment doesn't make sense, e.g., every computation you perform on typed registers. Alignment is only meaningful on memory operations and that is why LLVM IR decides to annotate the memory operations rather than the types.
That being said, we can have a default alignment attribute on types, but it should be disregarded for all computations over elements of that type. The alignment attribute should only influence the layout of composite types around the aligned type (say arrays and structs) and possibly the default alignment of allocations of that type (unless overwritten by the allocation site).
It will still yield ugly situations where, say, differently-aligned integers are added and you need to decide whose alignments wins (or compute a common one).

[hernanponcedeleon]

This only works if element has a basic type (e.g. int or bool). If it has an aggregate type, we should go deeper in the type hierarchy. I would start by throwing an exception in this case. The same is true for visitOpLoad.