farzonl wrote: > I'm still struggling to understand the motivation for this change and the > broader program of changes you keep bringing up. As far as I understand, > there are three separate issues at play which we seem to be conflating: > > 1. How matrix types are exposed in the Clang front-end (e.g., whether an > initialiser list is treated as row / column major); > 2. How Clang lays out matrix types in memory; > 3. How LLVM lowers use of matrix builtins to vector instructions. > > In my opinion, it does not make much sense for a compiler flag to impact 1, > and it makes perfect sense for both 2 and 3 to vary independently of 1 under > the hood. If row major memory layout is important and a priority, work to > implement the corresponding LLVM intrinsics should come first. It seems very > premature to add to, e.g., the release notes that we're allowing users to > control matrix memory layouts when nothing in the compiler has changed to > actually support these.
I think worrying about 1 right now is maybe getting a bit to ahead of ourselves since its not in this PR but I'll try to explain 1 a bit more. And maybe we take this discussion to discourse if my examples are not convincing. In DXC to control column vs row major we have the flags `/Zpc` and ` /Zpr` respectively Here is our 1st example https://hlsl.godbolt.org/z/8GE8b3o56 ``` RWStructuredBuffer<int> In; RWStructuredBuffer<int2x3> Out; [numthreads(1, 1, 1)] void CSMain(uint GI : SV_GroupIndex) { int2x3 M = int2x3(In[0], In[1], In[2], In[3], In[4], In[5]); Out[0] = M; } ``` If we look at the Diff we can see DXC today to support row/column major toggling has to wrap initalizers in the column major case as: - rowMatToColMat cast - colStore - colLoad - colMatToRowMat cast And in the row major case as: - rowStore - rowLoad <img width="3224" height="650" alt="image" src="https://github.com/user-attachments/assets/97024b7c-558b-4f27-8f56-44c95edf0aec"; /> The `rowMatToColMat` and `colMatToRowMat` casts are transformations I think can be avoided if we can impact things like the initalizer construction order. I've confirmed this via micro shader tests. Further its not how we treat an intialzer. We don't touch the initalizer list in any way. Its just how we consume it in the matrix constructor. From my observation from how DXC works today we do impact initalization consumption its just via casts and special load stores. The Second example is a matrix subscripting example: https://hlsl.godbolt.org/z/vT16MnPPv ```hlsl RWStructuredBuffer<int> In; RWStructuredBuffer<int> Out; cbuffer Constants { uint row; uint col; }; [numthreads(1, 1, 1)] void CSMain(uint GI : SV_GroupIndex) { int2x3 M = int2x3(In[0], In[1], In[2], In[3], In[4], In[5]); Out[0] = M[row][col]; } ``` I need to ability to change how subscript indexing works <img width="3218" height="572" alt="image" src="https://github.com/user-attachments/assets/1e13cee5-915d-44bb-88be-b906d3d2728e"; /> In the column major case this looks like so ``` /* DXC */ // Constructs M as a column-major order matrix M = [ In[0], In[3], In[1], In[4], In[2], In[5] ] // Computes a column-major flat index I = [ row, row + 2, row + 4 ] Out[0] = M[I[col]] // equivalent to M[row + col * 2] ``` To do this in a similar way I need to be able to modify `EmitMatrixSubscriptExpr` and `VisitMatrixSubscriptExpr` to do things like this: ```diff @@ -2118,9 +2118,17 @@ Value *ScalarExprEmitter::VisitMatrixSubscriptExpr(MatrixSubscriptExpr *E) { Value *ColumnIdx = CGF.EmitMatrixIndexExpr(E->getColumnIdx()); const auto *MatrixTy = E->getBase()->getType()->castAs<ConstantMatrixType>(); - unsigned NumRows = MatrixTy->getNumRows(); + llvm::MatrixBuilder MB(Builder); - Value *Idx = MB.CreateIndex(RowIdx, ColumnIdx, NumRows); + Value *Idx; + if (/*SOME_FLAG_FOR_ROW_MAJOR*/) { + unsigned NumCols = MatrixTy->getNumColumns(); + Idx = MB.createRowMajorIndex(RowIdx, ColumnIdx, NumCols); + } else { + unsigned NumRows = MatrixTy->getNumRows(); + Idx = MB.createColumnMajorIndex(RowIdx, ColumnIdx, NumRows); + } + ``` https://github.com/llvm/llvm-project/pull/167628 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
