[DL] Make vector ABI align bound by element align

[beanz]

Summary

If a data layout specifies a vector alignment that is less than the
alignment of the element type, prefer the element alignment.

This alters the vector ABI alignment so that vector elements are always
aligned appropriately for their type.

This oddity of data layout doesn't seem to happen in any existing
backends, but does in DirectX where vector alignment has a lower bound
of the element type. For example < 4 x float > requires 32-bit
alignment, and < 2 x double > requires 64.

[efriedma]

Whatever we do here should be documented in LangRef.

I'm a little wary of making assumptions like this implicitly. I think I'd prefer to make the datalayout string explicitly request this behavior.

llvm/include/llvm/Support/Alignment.h

218 ↗

(On Diff #458288)

Can you not just use std::max?

[tex3d]

I don't see why we would want a different behavior here between ABI and preferred alignments.

llvm/lib/IR/DataLayout.cpp

813

Shouldn't this capture the preferred alignment value for use in the maxAlignment, or return if not vector? Or is this mechanism intended to only apply to ABI alignment and not preferred alignment?

[beanz]

Updating based on review feedbac.

• Apply the behavior change to both ABI and Preferred alignments.
• Gate the change on a data layout variable (V).
• Update LangRef.rst documenting the new variable.

[jsilvanus]

This alters the vector ABI alignment so that vector elements are always aligned appropriately for their type.

How should this behave on overaligned element types? For example, with a i16:32 data layout (as in the old DXIL data layout),
the second element in <3 x i16> will not be 32-bit aligned, even if the vector is.

[arsenm]

Can dxil vectors be translated as arrays instead? Feels like someone interpreted vectors as directly mapping to a language language rather than a feature that doesn’t fit its needs.

[arsenm]

I have wanted to allow preferred alignments to be lower than the ABI alignments. Can we get per-type alignment specification instead of a global modifier?

[beanz]

In D133379#3962893, @jsilvanus wrote:

How should this behave on overaligned element types? For example, with a i16:32 data layout (as in the old DXIL data layout),
the second element in <3 x i16> will not be 32-bit aligned, even if the vector is.

16-bit types aren't supported with the old data layout at all. If you enable 16-bit types when compiling HLSL you get the new data layout which aligns 16-bit types to 16-bit boundaries. The only exception to that is some oddities around how buffers are packed, but that doesn't impact the data layout.

In D133379#3968986, @arsenm wrote:

Can dxil vectors be translated as arrays instead? Feels like someone interpreted vectors as directly mapping to a language language rather than a feature that doesn’t fit its needs.

DXIL is a shipped binary interface using IR vectors in this way. The current compiler does this without capturing the behavior in the data layout, which is something we'd like to change as we upstream the work.

In D133379#3968993, @arsenm wrote:

I have wanted to allow preferred alignments to be lower than the ABI alignments. Can we get per-type alignment specification instead of a global modifier?

Per-type specification makes sense, I can update the patch later this week.

[jsilvanus]

In D133379#3971623, @beanz wrote:

In D133379#3962893, @jsilvanus wrote:

How should this behave on overaligned element types? For example, with a i16:32 data layout (as in the old DXIL data layout),
the second element in <3 x i16> will not be 32-bit aligned, even if the vector is.

16-bit types aren't supported with the old data layout at all. If you enable 16-bit types when compiling HLSL you get the new data layout which aligns 16-bit types to 16-bit boundaries. The only exception to that is some oddities around how buffers are packed, but that doesn't impact the data layout.

Sure -- but still the behavior on such cases should be clear, even if it is not relevant for DXIL. I just stumbled over the formulation

This alters the vector ABI alignment so that vector elements are always aligned appropriately for their type.

that is promising too much in this case.

[beanz]

In D133379#3984033, @jsilvanus wrote:

Sure -- but still the behavior on such cases should be clear, even if it is not relevant for DXIL. I just stumbled over the formulation

I don't disagree. That wasn't the problem I was trying to solve with this change.

This alters the vector ABI alignment so that vector elements are always aligned appropriately for their type.

that is promising too much in this case.

Fair. That is probably an overly stated comment.