[Clang] [CG] Inconsistent lowering of matrix types to LLVM IR vector/array types

[Sirraide]

Currently, in ConvertTypeForMem(), we convert matrix types to LLVM IR array types, but in ConvertType(), we convert them to LLVM IR vector types. This doesn’t seem particularly sound to me since vector and array types afaik are generally not guaranteed to behave the same wrt padding etc. (and it is definitely not well-formed for e.g. _BitInt(12), but I think we should disallow that in any case; see #117487).

For example, consider (https://godbolt.org/z/EqjEq18Kz):

using mat3 = float [[clang::matrix_type(3, 3)]];
void f(mat3 x, mat3 y) { x * y; }

For f(), we emit:

define dso_local void @_Z1fu11matrix_typeILm3ELm3EfES_(<9 x float> noundef %x, <9 x float> noundef %y) #1 {
entry:
  %x.addr = alloca [9 x float], align 4
  %y.addr = alloca [9 x float], align 4
  store <9 x float> %x, ptr %x.addr, align 4
  store <9 x float> %y, ptr %y.addr, align 4
  %col.load = load <3 x float>, ptr %x.addr, align 4
  %vec.gep = getelementptr float, ptr %x.addr, i64 3
  ; ...

We should probably use vector types consistently considering that that’s what LLVM’s matrix intrinsics expect. We don’t support e.g. binding references to matrix subscript expressions, so it’s not like we really need them to be array types from what I can tell.

CC @efriedma-quic, @rjmccall, @fhahn

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: None (Sirraide)

Currently, in `ConvertTypeForMem()`, we convert matrix types to LLVM IR array types, but in `ConvertType()`, we convert them to LLVM IR vector types. This doesn’t seem particularly sound to me since vector and array types afaik are generally not guaranteed to behave the same wrt padding etc. (and it is *definitely* not well-formed for e.g. `_BitInt(12)`, but I think we should disallow that in any case; see #[INSERT_PR_NUMBER_HERE]).

For example, consider (https://godbolt.org/z/EqjEq18Kz):

using mat3 = float [[clang::matrix_type(3, 3)]];
void f(mat3 x, mat3 y) { x * y; }

For f(), we emit:

define dso_local void @<!-- -->_Z1fu11matrix_typeILm3ELm3EfES_(&lt;9 x float&gt; noundef %x, &lt;9 x float&gt; noundef %y) #<!-- -->1 {
entry:
  %x.addr = alloca [9 x float], align 4
  %y.addr = alloca [9 x float], align 4
  store &lt;9 x float&gt; %x, ptr %x.addr, align 4
  store &lt;9 x float&gt; %y, ptr %y.addr, align 4
  %col.load = load &lt;3 x float&gt;, ptr %x.addr, align 4
  %vec.gep = getelementptr float, ptr %x.addr, i64 3
  ; ...

We should probably use vector types consistently considering that that’s what LLVM’s matrix intrinsics expect. We don’t support e.g. binding references to matrix subscript expressions, so it’s not like we really need them to be array types from what I can tell.

CC @efriedma-quic, @rjmccall, @fhahn

[Sirraide]

Something similarly weird seems to be happening w/ vectors: #117487 (comment)

[rjmccall]

This is intentional; ConvertTypeForMem has to produce something that's got the right in-memory layout, and vector types generally do not because LLVM will round their size up according to a set of rules that likely does not match the ABI for matrix types. This is why we distinguish the scalar, in-memory, and load/store types in IR.

[Sirraide]

Ah, makes sense; in that case, I’m pretty sure the _BitInt case was the only problematic one, and that’s fixed now.