Emit directives to control matrix layout (#590)

The HLSL/GLSL output by Slang should try to be robust against whatever flags somebody uses to compile it. Therefore, we will go ahead and output a target-language-specific directive to control the default matrix layout mode so that we can override whatever might be specified via flags.

Also, as long as we are at it, this change goes ahead and makes Slang unconditionally emit row/column-major layout modifiers on all matrices (and arrays of matrices) whereas before these were only being output sometimes (the code to do it seemed buggy to me...).

1 files changed, 74 insertions, 8 deletions

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index af2fa68dc..e0f71aafe 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -979,7 +979,7 @@ struct EmitVisitor
         case CodeGenTarget::GLSL_Vulkan_OneDesc:
             {
                 // Need to special case if there is a single element in the vector
-                // as there is no such thing in glsl as vec1 
+                // as there is no such thing in glsl as vec1
                 IRInst* elementCountInst = vecType->getElementCount();
 
                 if (elementCountInst->op != kIROp_IntLit)
@@ -1914,6 +1914,68 @@ struct EmitVisitor
         // TODO: handle other cases...
     }
 
+    /// Emit directives to control overall layout computation for the emitted code.
+    void emitLayoutDirectives(TargetRequest* targetReq)
+    {
+        // We are going to emit the target-language-specific directives
+        // needed to get the default matrix layout to match what was requested
+        // for the given target.
+        //
+        // Note: we do not rely on the defaults for the target language,
+        // because a user could take the HLSL/GLSL generated by Slang and pass
+        // it to another compiler with non-default options specified on
+        // the command line, leading to all kinds of trouble.
+        //
+        // TODO: We need an approach to "global" layout directives that will work
+        // in the presence of multiple modules. If modules A and B were each
+        // compiled with different assumptions about how layout is performed,
+        // then types/variables defined in those modules should be emitted in
+        // a way that is consistent with that layout...
+
+        auto matrixLayoutMode = targetReq->defaultMatrixLayoutMode;
+
+        switch(context->shared->target)
+        {
+        default:
+            return;
+
+        case CodeGenTarget::GLSL:
+            // Reminder: the meaning of row/column major layout
+            // in our semantics is the *opposite* of what GLSL
+            // calls them, because what they call "columns"
+            // are what we call "rows."
+            //
+            switch(matrixLayoutMode)
+            {
+            case kMatrixLayoutMode_RowMajor:
+            default:
+                Emit("layout(column_major) uniform;\n");
+                Emit("layout(column_major) buffer;\n");
+                break;
+
+            case kMatrixLayoutMode_ColumnMajor:
+                Emit("layout(row_major) uniform;\n");
+                Emit("layout(row_major) buffer;\n");
+                break;
+            }
+            break;
+
+        case CodeGenTarget::HLSL:
+            switch(matrixLayoutMode)
+            {
+            case kMatrixLayoutMode_RowMajor:
+            default:
+                Emit("#pragma pack_matrix(row_major)\n");
+                break;
+
+            case kMatrixLayoutMode_ColumnMajor:
+                Emit("#pragma pack_matrix(column_major)\n");
+                break;
+            }
+            break;
+        }
+    }
+
     // Utility code for generating unique IDs as needed
     // during the emit process (e.g., for declarations
     // that didn't origianlly have names, but now need to).
@@ -4611,12 +4673,15 @@ struct EmitVisitor
         EmitContext*    ctx,
         VarLayout*      layout)
     {
-        // We need to handle the case where the variable has
-        // a matrix type, and has been given a non-standard
-        // layout attribute (for HLSL, `row_major` is the
-        // non-standard layout).
+        // When a variable has a matrix type, we want to emit an explicit
+        // layout qualifier based on what the layout has been computed to be.
         //
-        if (auto matrixTypeLayout = layout->typeLayout.As<MatrixTypeLayout>())
+
+        auto typeLayout = layout->typeLayout;
+        while(auto arrayTypeLayout = typeLayout.As<ArrayTypeLayout>())
+            typeLayout = arrayTypeLayout->elementTypeLayout;
+
+        if (auto matrixTypeLayout = typeLayout.As<MatrixTypeLayout>())
         {
             auto target = ctx->shared->target;
 
@@ -4626,7 +4691,6 @@ struct EmitVisitor
                 switch (matrixTypeLayout->mode)
                 {
                 case kMatrixLayoutMode_ColumnMajor:
-                    if(target == CodeGenTarget::GLSL)
                     emit("column_major ");
                     break;
 
@@ -4645,7 +4709,6 @@ struct EmitVisitor
                 switch (matrixTypeLayout->mode)
                 {
                 case kMatrixLayoutMode_ColumnMajor:
-                    if(target == CodeGenTarget::GLSL)
                     emit("layout(row_major)\n");
                     break;
 
@@ -5815,6 +5878,9 @@ String emitEntryPoint(
 
     // There may be global-scope modifiers that we should emit now
     visitor.emitGLSLPreprocessorDirectives(translationUnitSyntax);
+
+    visitor.emitLayoutDirectives(targetRequest);
+
     String prefix = sharedContext.sb.ProduceString();