[HLSL] Matrix constructors is using the initalizer index when it needs to convert the index to a column major index

[farzonl]

Today Matrix initalization is just taking in the index from the initalizer list:

llvm-project/clang/lib/Sema/SemaInit.cpp

Lines 1907 to 1913 in 15bbdd1

	while (NumEltsInit < MaxElts && Index < IList->getNumInits()) {
	// Not a sublist: just consume directly.
	ElemEnt.setElementIndex(Index);
	CheckSubElementType(ElemEnt, IList, ElemTy, Index, StructuredList,
	StructuredIndex);
	++NumEltsInit;
	}

This means we aren't reordering the index to account for layout DXC is expecting.

See https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-per-component-math#matrix-ordering

The fix seems to be

@@ -1906,10 +1906,12 @@ void InitListChecker::CheckMatrixType(const InitializedEntity &Entity,
 
   while (NumEltsInit < MaxElts && Index < IList->getNumInits()) {
     // Not a sublist: just consume directly.
-    ElemEnt.setElementIndex(Index);
-    CheckSubElementType(ElemEnt, IList, ElemTy, Index, StructuredList,
+    unsigned ColMajorIndex = (Index % MT->getNumRows()) * MT->getNumColumns() + (Index / MT->getNumRows());
+    ElemEnt.setElementIndex(ColMajorIndex);
+    CheckSubElementType(ElemEnt, IList, ElemTy, ColMajorIndex, StructuredList,
                         StructuredIndex);
     ++NumEltsInit;
+    ++Index;
   }

[Icohedron]

I don't think this is actually an issue.
Clang and DXC currently generate functionally-equivalent IR for matrix construction/initialization and subscripting/indexing.
DXC just transforms and loads elements from matrices in a way that makes things confusing.

Edit: I realize now the IR is actually not functionally equivalent. I notice Clang isn't consistent with using row- versus column-major ordering between matrix initialization and computing the flat index. In the below example, the matrix is initialized in row-major order, but the flat index being computed is row + col * 2 which is a flat index intended for a column-major order matrix.

https://hlsl.godbolt.org/z/57MnrTaYY

HLSL:

// compile args: -T lib_6_8
export int Foo(uint row, uint col) {
  int2x3 M = int2x3(0, 1, 2, 3, 4, 5);
  return M[row][col];
}

Clang:

define i32 @Foo(unsigned int, unsigned int)(i32 %0, i32 %1) local_unnamed_addr #0 !dbg !22 {
; Construct the matrix M (%3) from the elements given to the constructor in row-major order.
  %3 = alloca [6 x i32], align 4, !dbg !32
  %4 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 0, !dbg !32
  store i32 0, ptr %4, align 4, !dbg !32
  %5 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 1, !dbg !32
  store i32 1, ptr %5, align 4, !dbg !32
  %6 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 2, !dbg !32
  store i32 2, ptr %6, align 4, !dbg !32
  %7 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 3, !dbg !32
  store i32 3, ptr %7, align 4, !dbg !32
  %8 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 4, !dbg !32
  store i32 4, ptr %8, align 4, !dbg !32
  %9 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 5, !dbg !32
  store i32 5, ptr %9, align 4, !dbg !32
; Compute the index for the matrix: col * 2 + row == %1 * 2 + %0
  %10 = shl i32 %1, 1, !dbg !32
  %11 = add i32 %10, %0, !dbg !32
; Bounds check
  %12 = icmp ult i32 %11, 6, !dbg !32
  tail call void @llvm.assume(i1 %12), !dbg !32
; No-op address calculation from no-op GEP transformation.
  %13 = mul i32 %11, 1, !dbg !32
  %14 = add i32 0, %13, !dbg !32
; Load from the matrix at element col * 2 + row.
  %15 = getelementptr inbounds [6 x i32], ptr %3, i32 0, i32 %14, !dbg !32
  %16 = load i32, ptr %15, align 4, !dbg !32
  ret i32 %16, !dbg !33
}

DXC:

; Define a global constant holding the result of a row-major to column-major order transformation of the compile-time constant matrix M.
@.hca = internal unnamed_addr constant [6 x i32] [i32 0, i32 3, i32 1, i32 4, i32 2, i32 5]

define i32 @"\01?Foo@@YAHII@Z"(i32 %row, i32 %col) #0 {
; Construct an array of indices [ row, row + 2, row + 4 ]
  %1 = alloca [3 x i32], align 4
  %2 = add i32 %row, 2, !dbg !47 ; line:3 col:10
  %3 = add i32 %row, 4, !dbg !47 ; line:3 col:10
  %4 = getelementptr [3 x i32], [3 x i32]* %1, i32 0, i32 0, !dbg !47 ; line:3 col:10
  store i32 %row, i32* %4, align 4, !dbg !47 ; line:3 col:10
  %5 = getelementptr [3 x i32], [3 x i32]* %1, i32 0, i32 1, !dbg !47 ; line:3 col:10
  store i32 %2, i32* %5, align 4, !dbg !47 ; line:3 col:10
  %6 = getelementptr [3 x i32], [3 x i32]* %1, i32 0, i32 2, !dbg !47 ; line:3 col:10
  store i32 %3, i32* %6, align 4, !dbg !47 ; line:3 col:10
; Load load the col'th element from the array [ row, row + 2, row + 4] to get the index into the column-major version of the matrix M.
  %7 = getelementptr [3 x i32], [3 x i32]* %1, i32 0, i32 %col, !dbg !47 ; line:3 col:10
  %8 = load i32, i32* %7, align 4, !dbg !47 ; line:3 col:10
; Load the corresponding element from the column-major version of the matrix M using the index that was just loaded.
  %9 = getelementptr [6 x i32], [6 x i32]* @.hca, i32 0, i32 %8, !dbg !47 ; line:3 col:10
  %10 = load i32, i32* %9, align 4, !dbg !47 ; line:3 col:10
  ret i32 %10, !dbg !48 ; line:3 col:3
}

While in this example it may appear that DXC stores matrices in column-major order, it is not the case.
You can see in this example: https://godbolt.org/z/bzKeY91Ts
Where the matrix is from a function argument, DXC will create a copy of the matrix but with a row-major order to column-major order transformation, and then perform the same steps to index the column-order copy instead of the original row-major matrix from the function argument.

[farzonl]

Its not the store thats the issue its the indexing. I dont think you are interpreting what the array alloca of size 3 is doing correctly. The array is encoding base indices that should be the equivalent of row *3 + col

[Icohedron]

Indeed DXC is calculating row * 3 + col, which is the calculation for indexing into a row-major order 2x3 matrix.
It uses that to index into the row-major copy of the matrix M (not the original column-major order M).

Edit: I realize now both DXC and Clang are actually computing row + col * 2. The actual difference here is Clang is initializing the matrix M in row-major order, while DXC is initializing M in column-major order. Therefore it is actually Clang's initialization of the matrix M that is incorrect if we want DXC to be the spec.

[farzonl]

I indeed DXC is calculating row * 3 + col, which is the calculation for indexing into a row-major order matrix. It uses that to index into the row-major copy of the matrix M (not the original column-major order M).

But thats not what clang is doing today hence the bug. Below see the indexing differences:

row	col	DXC today and Clang after #165666 row * 3 + col	clang today col * 2 + row
0	0	0 * 3 + 0 = 0	0 * 2 + 0 = 0
0	1	0 * 3 + 1 = 1	1 * 2 + 0 = 2
0	2	0 * 3 + 2 = 2	2 * 2 + 0 = 4
1	0	1 * 3 + 0 = 3	0 * 2 + 1 = 1
1	1	1 * 3 + 1 = 4	1 * 2 + 1 = 3
1	2	1 * 3 + 2 = 5	2 * 2 + 1 = 5

[Icohedron]

Yes, but Clang is indexing into M itself (a column-major order matrix).
Meanwhile DXC is indexing into a row-major order copy of M.
That's why you see the indices differ.

DXC is introducing an intermediate row-major order copy of M for reasons I do not know.

Edit: Clang is actually initializing M in row-major order but indexing it with an index intended for a column-major order matrix.
DXC initialized M in column-major order and computes the index intended for a column-major order matrix but uses an extra array [row, row + 2, row + 4] instead of multiplying col by 2.

[farzonl]

Yes, but Clang is indexing into M itself. Meanwhile DXC is indexing into a row-major order copy of M. That's why you see the indices differ.

DXC is introducing an intermediate row-major order copy of M for reasons I do not know.

I'm not proposing we copy DXCs implementation. But DXCs behavior is by default the spec. We need an indexing scheme that matches it. I'll follow up with @llvm-beanz and @tex3d on the peculiarities.