Add options to control matrix layout rules (#583)

* Add options to control matrix layout rules Up to this point, the Slang compiler has assumed that the default matrix layout conventions for the target API will be used. This means column-major layout for D3D, and *row major* layout for GL/Vulkan (note that while GL/Vulkan describe the default as "column major" there is an implicit swap of "row" and "column" when mapping HLSL conventions to GLSL). This commit introduces two main changes: 1. The default layout convention is switched to column-major on all targets, to ensure that D3D and GL/Vulkan can easily be driven by the same application logic. I would prefer to make the default be row-major (because this is the "obvious" convention for matrices), but I don't want to deviate from the defaults in existing HLSL compilers. 2. Command-line and API options are introduced for setting the matrix layout convention to use (by default) for each code generation target. It is still possible for explicit qualifiers like `row_major` to change the layout from within shader code. I also added an API to query the matrix layout convention that was used for a type layout (which should be of the `SLANG_TYPE_KIND_MATRIX` kind), but this isn't yet exercised. I added a reflection test case to make sure that the offsets/sizes we compute for matrix-type fields are appropriately modified by the flag that gets passed in. In a future change we could possibly switch the default convention to row-major, if we also changed our testing to match, since there are currently not many clients to be adversely impacted by the change. * Fixup: silence 64-bit build warning
shader-slang · May 31, 2018 · 8d77db3 · 8d77db3
1 parent 8c593ae
commit 8d77db3
Show file tree

Hide file tree

Showing 10 changed files with 355 additions and 47 deletions.
diff --git a/slang.h b/slang.h
@@ -170,6 +170,14 @@ extern "C"
         SLANG_PROFILE_UNKNOWN,
     };
 
+    typedef unsigned int SlangMatrixLayoutMode;
+    enum
+    {
+        SLANG_MATRIX_LAYOUT_MODE_UNKNOWN = 0,
+        SLANG_MATRIX_LAYOUT_ROW_MAJOR,
+        SLANG_MATRIX_LAYOUT_COLUMN_MAJOR,
+    };
+
 //#define SLANG_LAYOUT_UNIFORM 0
 //#define SLANG_LAYOUT_PACKED 1
 //#define SLANG_LAYOUT_STORAGE 2
@@ -271,6 +279,11 @@ extern "C"
         int                     targetIndex,
         SlangTargetFlags        flags);
 
+    SLANG_API void spSetTargetMatrixLayoutMode(
+        SlangCompileRequest*    request,
+        int                     targetIndex,
+        SlangMatrixLayoutMode   mode);
+
     /*!
     @brief Set the container format to be used for binary output.
     */
@@ -683,6 +696,8 @@ extern "C"
     SLANG_API unsigned spReflectionTypeLayout_GetCategoryCount(SlangReflectionTypeLayout* type);
     SLANG_API SlangParameterCategory spReflectionTypeLayout_GetCategoryByIndex(SlangReflectionTypeLayout* type, unsigned index);
 
+    SLANG_API SlangMatrixLayoutMode spReflectionTypeLayout_GetMatrixLayoutMode(SlangReflectionTypeLayout* type);
+
     // Variable Reflection
 
     SLANG_API char const* spReflectionVariable_GetName(SlangReflectionVariable* var);
@@ -1039,6 +1054,12 @@ namespace slang
         {
             return getType()->getName();
         }
+
+        SlangMatrixLayoutMode getMatrixLayoutMode()
+        {
+            return spReflectionTypeLayout_GetMatrixLayoutMode((SlangReflectionTypeLayout*) this);
+        }
+
     };
 
     struct Modifier

diff --git a/source/slang/compiler.h b/source/slang/compiler.h
@@ -73,6 +73,18 @@ namespace Slang
         Binary
     };
 
+    // When storing the layout for a matrix-type
+    // value, we need to know whether it has been
+    // laid ot with row-major or column-major
+    // storage.
+    //
+    enum MatrixLayoutMode
+    {
+        kMatrixLayoutMode_RowMajor      = SLANG_MATRIX_LAYOUT_ROW_MAJOR,
+        kMatrixLayoutMode_ColumnMajor   = SLANG_MATRIX_LAYOUT_COLUMN_MAJOR,
+    };
+
+
     class CompileRequest;
     class TranslationUnitRequest;
 
@@ -221,6 +233,10 @@ namespace Slang
 
         // TypeLayouts created on the fly by reflection API
         Dictionary<Type*, RefPtr<TypeLayout>> typeLayouts;
+
+        /// The layout to use for matrices by default (row/column major)
+        MatrixLayoutMode defaultMatrixLayoutMode = kMatrixLayoutMode_ColumnMajor;
+        MatrixLayoutMode getDefaultMatrixLayoutMode() { return defaultMatrixLayoutMode; }
     };
 
     // Compute the "effective" profile to use when outputting the given entry point

diff --git a/source/slang/options.cpp b/source/slang/options.cpp
@@ -247,6 +247,8 @@ struct OptionsParser
 
         //
 
+        SlangMatrixLayoutMode defaultMatrixLayoutMode = SLANG_MATRIX_LAYOUT_MODE_UNKNOWN;
+
         char const* const* argCursor = &argv[0];
         char const* const* argEnd = &argv[argc];
         while (argCursor != argEnd)
@@ -488,6 +490,14 @@ struct OptionsParser
 
                     addOutputPath(outputPath);
                 }
+                else if(argStr == "-matrix-layout-row-major")
+                {
+                    defaultMatrixLayoutMode = kMatrixLayoutMode_RowMajor;
+                }
+                else if(argStr == "-matrix-layout-column-major")
+                {
+                    defaultMatrixLayoutMode = kMatrixLayoutMode_ColumnMajor;
+                }
                 else if (argStr == "--")
                 {
                     // The `--` option causes us to stop trying to parse options,
@@ -745,6 +755,15 @@ struct OptionsParser
             spSetTargetFlags(compileRequest, 0, targetFlags);
         }
 
+        if(defaultMatrixLayoutMode != SLANG_MATRIX_LAYOUT_MODE_UNKNOWN)
+        {
+            UInt targetCount = requestImpl->targets.Count();
+            for(UInt tt = 0; tt < targetCount; ++tt)
+            {
+                spSetTargetMatrixLayoutMode(compileRequest, int(tt), defaultMatrixLayoutMode);
+            }
+        }
+
         // Next, we want to make sure that entry points get attached to the appropriate translation
         // unit that will provide them.
         {

diff --git a/source/slang/reflection.cpp b/source/slang/reflection.cpp
@@ -658,6 +658,23 @@ SLANG_API SlangParameterCategory spReflectionTypeLayout_GetCategoryByIndex(Slang
     return typeLayout->resourceInfos[index].kind;
 }
 
+SLANG_API SlangMatrixLayoutMode spReflectionTypeLayout_GetMatrixLayoutMode(SlangReflectionTypeLayout* inTypeLayout)
+{
+    auto typeLayout = convert(inTypeLayout);
+    if(!typeLayout) return SLANG_MATRIX_LAYOUT_MODE_UNKNOWN;
+
+    if( auto matrixLayout = dynamic_cast<MatrixTypeLayout*>(typeLayout) )
+    {
+        return matrixLayout->mode;
+    }
+    else
+    {
+        return SLANG_MATRIX_LAYOUT_MODE_UNKNOWN;
+    }
+
+}
+
+
 // Variable Reflection
 
 SLANG_API char const* spReflectionVariable_GetName(SlangReflectionVariable* inVar)

diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
@@ -952,6 +952,16 @@ SLANG_API void spSetTargetFlags(
     req->targets[targetIndex]->targetFlags = flags;
 }
 
+SLANG_API void spSetTargetMatrixLayoutMode(
+    SlangCompileRequest*    request,
+    int                     targetIndex,
+    SlangMatrixLayoutMode   mode)
+{
+    auto req = REQ(request);
+    req->targets[targetIndex]->defaultMatrixLayoutMode = Slang::MatrixLayoutMode(mode);
+}
+
+
 SLANG_API void spSetOutputContainerFormat(
     SlangCompileRequest*    request,
     SlangContainerFormat    format)

diff --git a/source/slang/type-layout.cpp b/source/slang/type-layout.cpp
@@ -27,12 +27,6 @@ static size_t RoundUpToPowerOfTwo( size_t value )
 
 //
 
-MatrixLayoutMode LayoutRulesImpl::getDefaultMatrixLayoutMode() {
-    return family->getDefaultMatrixLayoutMode();
-}
-
-//
-
 struct DefaultLayoutRulesImpl : SimpleLayoutRulesImpl
 {
     // Get size and alignment for a single value of base type.
@@ -428,25 +422,6 @@ struct GLSLLayoutRulesFamilyImpl : LayoutRulesFamilyImpl
     virtual LayoutRulesImpl* getSpecializationConstantRules() override;
     virtual LayoutRulesImpl* getShaderStorageBufferRules() override;
     virtual LayoutRulesImpl* getParameterBlockRules() override;
-
-    virtual MatrixLayoutMode getDefaultMatrixLayoutMode() override
-    {
-        // The default matrix layout mode in GLSL is specified
-        // to be "column major" but what GLSL calls a "column"
-        // is actually what HLSL (and hence Slang) calls a row.
-        //
-        // That is, an HLSL `float3x4` has 3 rows and 4 columns,
-        // and indexing yields a `float4`.
-        //
-        // A GLSL `mat3x4` has 3 "columns" and 4 "rows", and
-        // indexing into it yields a `vec4`.
-        //
-        // The Slang compiler needs to be consistent about this mess,
-        // and so when the GLSL spec says that "column"-major is
-        // the default, we know that they actually mean what we
-        // call row-major.
-        return kMatrixLayoutMode_RowMajor;
-    }
 };
 
 struct HLSLLayoutRulesFamilyImpl : LayoutRulesFamilyImpl
@@ -459,11 +434,6 @@ struct HLSLLayoutRulesFamilyImpl : LayoutRulesFamilyImpl
     virtual LayoutRulesImpl* getSpecializationConstantRules() override;
     virtual LayoutRulesImpl* getShaderStorageBufferRules() override;
     virtual LayoutRulesImpl* getParameterBlockRules() override;
-
-    virtual MatrixLayoutMode getDefaultMatrixLayoutMode() override
-    {
-        return kMatrixLayoutMode_ColumnMajor;
-    }
 };
 
 GLSLLayoutRulesFamilyImpl kGLSLLayoutRulesFamilyImpl;
@@ -644,12 +614,11 @@ TypeLayoutContext getInitialLayoutContextForTarget(TargetRequest* targetReq)
     TypeLayoutContext context;
     context.targetReq = targetReq;
     context.rules = nullptr;
-    context.matrixLayoutMode = MatrixLayoutMode::kMatrixLayoutMode_RowMajor;
+    context.matrixLayoutMode = targetReq->getDefaultMatrixLayoutMode();
 
     if( rulesFamily )
     {
         context.rules = rulesFamily->getConstantBufferRules();
-        context.matrixLayoutMode = rulesFamily->getDefaultMatrixLayoutMode();
     }
 
     return context;
@@ -1150,7 +1119,7 @@ createParameterGroupTypeLayout(
     RefPtr<TypeLayout>          elementTypeLayout)
 {
     return createParameterGroupTypeLayout(
-        context.with(parameterGroupRules).with(parameterGroupRules->getDefaultMatrixLayoutMode()),
+        context.with(parameterGroupRules).with(context.targetReq->getDefaultMatrixLayoutMode()),
         parameterGroupType,
         parameterGroupInfo,
         elementTypeLayout);

diff --git a/source/slang/type-layout.h b/source/slang/type-layout.h
@@ -361,16 +361,6 @@ class StreamOutputTypeLayout : public TypeLayout
 };
 
 
-// When storing the layout for a matrix-type
-// value, we need to know whether it has been
-// laid ot with row-major or column-major
-// storage.
-//
-enum MatrixLayoutMode
-{
-    kMatrixLayoutMode_RowMajor,
-    kMatrixLayoutMode_ColumnMajor,
-};
 class MatrixTypeLayout : public TypeLayout
 {
 public:
@@ -603,8 +593,6 @@ struct LayoutRulesImpl
     //
 
     LayoutRulesFamilyImpl* getLayoutRulesFamily() { return family; }
-
-    MatrixLayoutMode getDefaultMatrixLayoutMode();
 };
 
 struct LayoutRulesFamilyImpl
@@ -617,8 +605,6 @@ struct LayoutRulesFamilyImpl
     virtual LayoutRulesImpl* getSpecializationConstantRules()= 0;
     virtual LayoutRulesImpl* getShaderStorageBufferRules()  = 0;
     virtual LayoutRulesImpl* getParameterBlockRules()       = 0;
-
-    virtual MatrixLayoutMode getDefaultMatrixLayoutMode() = 0;
 };
 
 struct TypeLayoutContext

diff --git a/tests/reflection/matrix-layout.slang b/tests/reflection/matrix-layout.slang
@@ -0,0 +1,28 @@
+//TEST:REFLECTION:-profile ps_4_0 -target hlsl
+//TEST:REFLECTION:-profile ps_4_0 -target hlsl -matrix-layout-row-major
+
+// Test that we apply matrix layout rules correctly.
+
+cbuffer A
+{
+					float3x4 aa;
+	row_major 		float3x4 ab;
+	column_major 	float3x4 ac;
+}
+
+struct SB
+{
+					float3x4 ba;
+	row_major 		float3x4 bb;
+	column_major 	float3x4 bc;
+};
+
+cbuffer B
+{
+	SB b;
+}
+
+float4 main() : SV_Target
+{
+	return 0.0;
+}