[spirv] Wrap global matrix variables in structs (#1050)

According to HLSL doc, "variables that are placed in the global scope
are added implicitly to the $Global cbuffer, using the same packing
method that is used for cbuffers."

But we emit all global variables as stand-alone SPIR-V variables.
This causes issues for matrix variables since we cannot annotate
them with majorness decoration anymore.

Wrap global matrix variables in a struct to solve the problem.

docs/SPIR-V.rst

@@ -723,6 +723,22 @@ HLSL Variables and Resources
 
 This section lists how various HLSL variables and resources are mapped.
 
+According to `Shader Constants <https://msdn.microsoft.com/en-us/library/windows/desktop/bb509581(v=vs.85).aspx>`_,
+
+  There are two default constant buffers available, $Global and $Param. Variables
+  that are placed in the global scope are added implicitly to the $Global cbuffer,
+  using the same packing method that is used for cbuffers. Uniform parameters in
+  the parameter list of a function appear in the $Param constant buffer when a
+  shader is compiled outside of the effects framework.
+
+However, when targeting SPIR-V, all externally visible variables are translated
+into stand-alone SPIR-V variables of their original types; they are not grouped
+together into a struct. There is one exception regarding matrix variables,
+though. For an externally visible matrix, we wrap it in a struct; the struct has
+no other members but the matrix. The reason of this behavior is to enable
+translating the ``row_major``/``column_major`` annotation since SPIR-V only
+allows ``RowMajor``/``ColMajor`` decorations to appear on struct members.
+
 Storage class
 -------------
 
@@ -2500,7 +2516,7 @@ Notice that in HLSL, ``m[0]`` represents a row, which is a vector of size 3. But
 the first column which is a vector of size 2.
 
 .. code:: spirv
-  
+
   ; n is a vector of size 2
   %n = OpAccessChain %v2float %m %int_0
 

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -383,11 +383,21 @@ uint32_t DeclResultIdMapper::createFileVar(const VarDecl *var,
 uint32_t DeclResultIdMapper::createExternVar(const VarDecl *var) {
   auto storageClass = spv::StorageClass::UniformConstant;
   auto rule = LayoutRule::Void;
+  bool isMatType = false;     // Whether this var is of matrix type
   bool isACRWSBuffer = false; // Whether its {Append|Consume|RW}StructuredBuffer
 
   if (var->getAttr<HLSLGroupSharedAttr>()) {
     // For CS groupshared variables
     storageClass = spv::StorageClass::Workgroup;
+  } else if (TypeTranslator::isMxNMatrix(var->getType())) {
+    isMatType = true;
+    // According to HLSL doc:
+    //   Variables that are placed in the global scope are added implicitly to
+    //   the $Global cbuffer, using the same packing method that is used for
+    //   cbuffers.
+    // So we should translate stand-alone matrices like cbuffer.
+    storageClass = spv::StorageClass::Uniform;
+    rule = LayoutRule::GLSLStd140;
   } else if (auto *t = var->getType()->getAs<RecordType>()) {
     const llvm::StringRef typeName = t->getDecl()->getName();
 
@@ -407,11 +417,25 @@ uint32_t DeclResultIdMapper::createExternVar(const VarDecl *var) {
     }
   }
 
-  const auto varType = typeTranslator.translateType(var->getType(), rule);
+  uint32_t varType = 0;
+
+  if (isMatType) {
+    // For stand-alone matrices, we need to wrap it in a struct so that we can
+    // annotate the majorness decoration.
+    varType = getMatrixStructType(var, storageClass, rule);
+  } else {
+    varType = typeTranslator.translateType(var->getType(), rule);
+  }
+
   const uint32_t id = theBuilder.addModuleVar(varType, storageClass,
                                               var->getName(), llvm::None);
   astDecls[var] =
       SpirvEvalInfo(id).setStorageClass(storageClass).setLayoutRule(rule);
+  if (isMatType) {
+    // We have wrapped the stand-alone matrix inside a struct. Mark it as
+    // needing an extra index to access.
+    astDecls[var].indexInCTBuffer = 0;
+  }
 
   const auto *regAttr = getResourceBinding(var);
   const auto *bindingAttr = var->getAttr<VKBindingAttr>();
@@ -432,6 +456,30 @@ uint32_t DeclResultIdMapper::createExternVar(const VarDecl *var) {
   return id;
 }
 
+uint32_t DeclResultIdMapper::getMatrixStructType(const VarDecl *matVar,
+                                                 spv::StorageClass sc,
+                                                 LayoutRule rule) {
+  const auto matType = matVar->getType();
+  assert(TypeTranslator::isMxNMatrix(matType));
+
+  auto &context = *theBuilder.getSPIRVContext();
+  llvm::SmallVector<const Decoration *, 4> decorations;
+  const bool isRowMajor = typeTranslator.isRowMajorMatrix(matType, matVar);
+
+  uint32_t stride;
+  (void)typeTranslator.getAlignmentAndSize(matType, rule, isRowMajor, &stride);
+  decorations.push_back(Decoration::getOffset(context, 0, 0));
+  decorations.push_back(Decoration::getMatrixStride(context, stride, 0));
+  decorations.push_back(isRowMajor ? Decoration::getColMajor(context, 0)
+                                   : Decoration::getRowMajor(context, 0));
+  decorations.push_back(Decoration::getBlock(context));
+
+  // Get the type for the wrapping struct
+  const std::string structName = "type." + matVar->getName().str();
+  return theBuilder.getStructType({typeTranslator.translateType(matType)},
+                                  structName, {}, decorations);
+}
+
 uint32_t DeclResultIdMapper::createVarOfExplicitLayoutStruct(
     const DeclContext *decl, const ContextUsageKind usageKind,
     llvm::StringRef typeName, llvm::StringRef varName) {

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -493,6 +493,11 @@ private:
   /// construction.
   bool finalizeStageIOLocations(bool forInput);
 
+  /// \brief Wraps the given matrix type with a struct and returns the struct
+  /// type's <result-id>.
+  uint32_t getMatrixStructType(const VarDecl *matVar, spv::StorageClass,
+                               LayoutRule);
+
   /// \brief An enum class for representing what the DeclContext is used for
   enum class ContextUsageKind {
     CBuffer,

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -965,16 +965,6 @@ void SPIRVEmitter::doFunctionDecl(const FunctionDecl *decl) {
 bool SPIRVEmitter::validateVKAttributes(const NamedDecl *decl) {
   bool success = true;
 
-  if (decl->hasAttr<HLSLRowMajorAttr>()) {
-    emitWarning("row_major attribute for stand-alone matrix is not supported",
-                decl->getAttr<HLSLRowMajorAttr>()->getLocation());
-  }
-  if (decl->hasAttr<HLSLColumnMajorAttr>()) {
-    emitWarning(
-        "column_major attribute for stand-alone matrix is not supported",
-        decl->getAttr<HLSLColumnMajorAttr>()->getLocation());
-  }
-
   if (const auto *varDecl = dyn_cast<VarDecl>(decl)) {
     const auto varType = varDecl->getType();
     if ((TypeTranslator::isSubpassInput(varType) ||

tools/clang/test/CodeGenSPIRV/type.matrix.majorness.hlsl

@@ -1,18 +0,0 @@
-// Run: %dxc -T ps_6_0 -E main
-
-// CHECK: 4:1: warning: row_major attribute for stand-alone matrix is not supported
-row_major float2x3 grMajorMat;
-// CHECK: 6:1: warning: column_major attribute for stand-alone matrix is not supported
-column_major float2x3 gcMajorMat;
-
-// CHECK: 9:8: warning: row_major attribute for stand-alone matrix is not supported
-static row_major float2x3 gsrMajorMat;
-// CHECK: 11:8: warning: column_major attribute for stand-alone matrix is not supported
-static column_major float2x3 gscMajorMat;
-
-void main() {
-  // CHECK: 15:3: warning: row_major attribute for stand-alone matrix is not supported
-  row_major float2x3 rMajorMat;
-  // CHECK: 17:3: warning: column_major attribute for stand-alone matrix is not supported
-  column_major float2x3 cMajorMat;
-}

tools/clang/test/CodeGenSPIRV/var.global-mat.hlsl

@@ -0,0 +1,32 @@
+// Run: %dxc -T vs_6_0 -E main
+
+// CHECK:      OpMemberDecorate %type_gMat1 0 Offset 0
+// CHECK-NEXT: OpMemberDecorate %type_gMat1 0 MatrixStride 16
+// CHECK-NEXT: OpMemberDecorate %type_gMat1 0 RowMajor
+// CHECK-NEXT: OpDecorate %type_gMat1 Block
+
+// CHECK:      OpMemberDecorate %type_gMat2 0 Offset 0
+// CHECK-NEXT: OpMemberDecorate %type_gMat2 0 MatrixStride 16
+// CHECK-NEXT: OpMemberDecorate %type_gMat2 0 ColMajor
+// CHECK-NEXT: OpDecorate %type_gMat2 Block
+
+// CHECK: %gMat1 = OpVariable %_ptr_Uniform_type_gMat1 Uniform
+// CHECK: %gMat2 = OpVariable %_ptr_Uniform_type_gMat2 Uniform
+// CHECK: %gMat3 = OpVariable %_ptr_Uniform_type_gMat1 Uniform
+             float2x3 gMat1;
+row_major    float2x3 gMat2;
+column_major float2x3 gMat3;
+
+void main() {
+// CHECK:      [[mat:%\d+]] = OpAccessChain %_ptr_Uniform_mat2v3float %gMat1 %int_0
+// CHECK-NEXT:     {{%\d+}} = OpLoad %mat2v3float [[mat]]
+    float2x3 mat = gMat1;
+
+// CHECK:      [[mat:%\d+]] = OpAccessChain %_ptr_Uniform_mat2v3float %gMat2 %int_0
+// CHECK-NEXT:     {{%\d+}} = OpAccessChain %_ptr_Uniform_v3float [[mat]] %uint_0
+    float3   vec = gMat2[0];
+
+// CHECK:      [[mat:%\d+]] = OpAccessChain %_ptr_Uniform_mat2v3float %gMat3 %int_0
+// CHECK-NEXT:     {{%\d+}} = OpAccessChain %_ptr_Uniform_float [[mat]] %int_0 %int_0
+    float scalar = gMat3._11;
+}

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -51,9 +51,6 @@ TEST_F(FileTest, MatrixTypesMajornessZpr) {
 TEST_F(FileTest, MatrixTypesMajornessZpc) {
   runFileTest("type.matrix.majorness.zpc.hlsl");
 }
-TEST_F(FileTest, MatrixTypesMajorness) {
-  runFileTest("type.matrix.majorness.hlsl", Expect::Warning);
-}
 TEST_F(FileTest, StructTypes) { runFileTest("type.struct.hlsl"); }
 TEST_F(FileTest, ClassTypes) { runFileTest("type.class.hlsl"); }
 TEST_F(FileTest, ArrayTypes) { runFileTest("type.array.hlsl"); }
@@ -123,6 +120,7 @@ TEST_F(FileTest, VarInitCrossStorageClass) {
   runFileTest("var.init.cross-storage-class.hlsl");
 }
 TEST_F(FileTest, StaticVar) { runFileTest("var.static.hlsl"); }
+TEST_F(FileTest, GlobalMatrixVar) { runFileTest("var.global-mat.hlsl"); }
 
 // For prefix/postfix increment/decrement
 TEST_F(FileTest, UnaryOpPrefixIncrement) {