[spirv] Handle TessFactor size mismatches. (#771)

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -765,13 +765,15 @@ bool DeclResultIdMapper::createStageVars(
   uint32_t semanticIndex = {};
 
   if (getStageVarSemantic(decl, &semanticStr, &semantic, &semanticIndex)) {
+    const auto semanticKind = semantic->GetKind();
+
     // Found semantic attached directly to this Decl. This means we need to
     // map this decl to a single stage variable.
 
     // Error out when the given semantic is invalid in this shader model
-    if (hlsl::SigPoint::GetInterpretation(
-            semantic->GetKind(), sigPoint->GetKind(), shaderModel.GetMajor(),
-            shaderModel.GetMinor()) ==
+    if (hlsl::SigPoint::GetInterpretation(semanticKind, sigPoint->GetKind(),
+                                          shaderModel.GetMajor(),
+                                          shaderModel.GetMinor()) ==
         hlsl::DXIL::SemanticInterpretationKind::NA) {
       emitError("invalid semantic %0 for shader model %1")
           << semanticStr << shaderModel.GetName();
@@ -785,12 +787,24 @@ bool DeclResultIdMapper::createStageVars(
     // * SV_DomainLocation can refer to a float2, whereas TessCoord is a float3.
     //   To ensure SPIR-V validity, we must create a float3 and  extract a
     //   float2 from it before passing it to the main function.
-    if (glPerVertex.tryToAccess(semantic->GetKind(), semanticIndex,
-                                invocationId, value, sigPoint->GetKind()))
+    // * SV_TessFactor is an array of size 2 for isoline patch, array of size 3
+    //   for tri patch, and array of size 4 for quad patch, but it must always
+    //   be an array of size 4 in SPIR-V for Vulkan.
+    // * SV_InsideTessFactor is a single float for tri patch, and an array of
+    //   size 2 for a quad patch, but it must always be an array of size 2 in
+    //   SPIR-V for Vulkan.
+    if (glPerVertex.tryToAccess(semanticKind, semanticIndex, invocationId,
+                                value, sigPoint->GetKind()))
       return true;
 
-    if (semantic->GetKind() == hlsl::Semantic::Kind::DomainLocation)
+    if (semanticKind == hlsl::Semantic::Kind::DomainLocation)
       typeId = theBuilder.getVecType(theBuilder.getFloat32Type(), 3);
+    else if (semanticKind == hlsl::Semantic::Kind::TessFactor)
+      typeId = theBuilder.getArrayType(theBuilder.getFloat32Type(),
+                                       theBuilder.getConstantUint32(4));
+    else if (semanticKind == hlsl::Semantic::Kind::InsideTessFactor)
+      typeId = theBuilder.getArrayType(theBuilder.getFloat32Type(),
+                                       theBuilder.getConstantUint32(2));
 
     // Handle the extra arrayness
     const uint32_t elementTypeId = typeId;
@@ -820,17 +834,93 @@ bool DeclResultIdMapper::createStageVars(
 
     if (asInput) {
       *value = theBuilder.createLoad(typeId, varId);
+
+      // Fix ups for corner cases
+
+      // Special handling of SV_TessFactor DS patch constant input.
+      // TessLevelOuter is always an array of size 4 in SPIR-V, but
+      // SV_TessFactor could be an array of size 2, 3, or 4 in HLSL. Only the
+      // relevant indexes must be loaded.
+      if (semanticKind == hlsl::Semantic::Kind::TessFactor &&
+          hlsl::GetArraySize(type) != 4) {
+        llvm::SmallVector<uint32_t, 4> components;
+        const auto f32TypeId = theBuilder.getFloat32Type();
+        const auto tessFactorSize = hlsl::GetArraySize(type);
+        const auto arrType = theBuilder.getArrayType(
+            f32TypeId, theBuilder.getConstantUint32(tessFactorSize));
+        for (uint32_t i = 0; i < tessFactorSize; ++i)
+          components.push_back(
+              theBuilder.createCompositeExtract(f32TypeId, *value, {i}));
+        *value = theBuilder.createCompositeConstruct(arrType, components);
+      }
+      // Special handling of SV_InsideTessFactor DS patch constant input.
+      // TessLevelInner is always an array of size 2 in SPIR-V, but
+      // SV_InsideTessFactor could be an array of size 1 (scalar) or size 2 in
+      // HLSL. If SV_InsideTessFactor is a scalar, only extract index 0 of
+      // TessLevelInner.
+      else if (semanticKind == hlsl::Semantic::Kind::InsideTessFactor &&
+               !type->isArrayType()) {
+        *value = theBuilder.createCompositeExtract(theBuilder.getFloat32Type(),
+                                                   *value, {0});
+      }
+      // SV_DomainLocation can refer to a float2 or a float3, whereas TessCoord
+      // is always a float3. To ensure SPIR-V validity, a float3 stage variable
+      // is created, and we must extract a float2 from it before passing it to
+      // the main function.
+      else if (semanticKind == hlsl::Semantic::Kind::DomainLocation &&
+               hlsl::GetHLSLVecSize(type) != 3) {
+        const auto domainLocSize = hlsl::GetHLSLVecSize(type);
+        *value = theBuilder.createVectorShuffle(
+            theBuilder.getVecType(theBuilder.getFloat32Type(), domainLocSize),
+            *value, *value, {0, 1});
+      }
     } else {
       uint32_t ptr = varId;
-      if (invocationId.hasValue()) {
-        // Special handling of HS ouput, for which we write to only one element
-        // in the per-vertex data array: the one indexed by  SV_ControlPointID.
+
+      // Special handling of SV_TessFactor HS patch constant output.
+      // TessLevelOuter is always an array of size 4 in SPIR-V, but
+      // SV_TessFactor could be an array of size 2, 3, or 4 in HLSL. Only the
+      // relevant indexes must be written to.
+      if (semanticKind == hlsl::Semantic::Kind::TessFactor &&
+          hlsl::GetArraySize(type) != 4) {
+        const auto f32TypeId = theBuilder.getFloat32Type();
+        const auto tessFactorSize = hlsl::GetArraySize(type);
+        for (uint32_t i = 0; i < tessFactorSize; ++i) {
+          const uint32_t ptrType =
+              theBuilder.getPointerType(f32TypeId, spv::StorageClass::Output);
+          ptr = theBuilder.createAccessChain(ptrType, varId,
+                                             theBuilder.getConstantUint32(i));
+          theBuilder.createStore(
+              ptr, theBuilder.createCompositeExtract(f32TypeId, *value, i));
+        }
+      }
+      // Special handling of SV_InsideTessFactor HS patch constant output.
+      // TessLevelInner is always an array of size 2 in SPIR-V, but
+      // SV_InsideTessFactor could be an array of size 1 (scalar) or size 2 in
+      // HLSL. If SV_InsideTessFactor is a scalar, only write to index 0 of
+      // TessLevelInner.
+      else if (semanticKind == hlsl::Semantic::Kind::InsideTessFactor &&
+               !type->isArrayType()) {
+        ptr = theBuilder.createAccessChain(
+            theBuilder.getPointerType(theBuilder.getFloat32Type(),
+                                      spv::StorageClass::Output),
+            varId, theBuilder.getConstantUint32(0));
+        theBuilder.createStore(ptr, *value);
+      }
+      // Special handling of HS ouput, for which we write to only one
+      // element in the per-vertex data array: the one indexed by
+      // SV_ControlPointID.
+      else if (invocationId.hasValue()) {
         const uint32_t ptrType =
             theBuilder.getPointerType(elementTypeId, spv::StorageClass::Output);
         const uint32_t index = invocationId.getValue();
         ptr = theBuilder.createAccessChain(ptrType, varId, index);
+        theBuilder.createStore(ptr, *value);
+      }
+      // For all normal cases
+      else {
+        theBuilder.createStore(ptr, *value);
       }
-      theBuilder.createStore(ptr, *value);
     }
 
     return true;

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -5767,15 +5767,6 @@ bool SPIRVEmitter::emitEntryFunctionWrapper(const FunctionDecl *decl,
       if (!declIdMapper.createStageInputVar(param, &loadedValue, false))
         return false;
 
-      // SV_DomainLocation can refer to a float2, whereas TessCoord is a float3.
-      // To ensure SPIR-V validity, a float3 stage variable is created, and we
-      // must extract a float2 from it before passing it to the main function.
-      if (hasSemantic(param, hlsl::DXIL::SemanticKind::DomainLocation) &&
-          hlsl::GetHLSLVecSize(param->getType()) == 2) {
-        loadedValue = theBuilder.createVectorShuffle(typeId, loadedValue,
-                                                     loadedValue, {0, 1});
-      }
-
       theBuilder.createStore(tempVar, loadedValue);
 
       // Record the temporary variable holding SV_OutputControlPointID and

tools/clang/test/CodeGenSPIRV/bezier.hull.hlsl2spv

@@ -44,7 +44,7 @@ HS_CONSTANT_DATA_OUTPUT SubDToBezierConstantsHS(InputPatch<VS_CONTROL_POINT_OUTP
   return Output;
 }
 
-[domain("tri")]
+[domain("quad")]
 [partitioning("fractional_odd")]
 [outputtopology("triangle_ccw")]
 [outputcontrolpoints(MAX_POINTS)]
@@ -60,7 +60,7 @@ BEZIER_CONTROL_POINT SubDToBezierHS(InputPatch<VS_CONTROL_POINT_OUTPUT, MAX_POIN
 // OpCapability Tessellation
 // OpMemoryModel Logical GLSL450
 // OpEntryPoint TessellationControl %SubDToBezierHS "SubDToBezierHS" %gl_PerVertexIn %gl_PerVertexOut %in_var_WORLDPOS %in_var_TEXCOORD0 %in_var_TANGENT %gl_InvocationID %gl_PrimitiveID %out_var_BEZIERPOS %gl_TessLevelOuter %gl_TessLevelInner %out_var_TANGENT %out_var_TEXCOORD %out_var_TANUCORNER %out_var_TANVCORNER %out_var_TANWEIGHTS
-// OpExecutionMode %SubDToBezierHS Triangles
+// OpExecutionMode %SubDToBezierHS Quads
 // OpExecutionMode %SubDToBezierHS SpacingFractionalOdd
 // OpExecutionMode %SubDToBezierHS VertexOrderCcw
 // OpExecutionMode %SubDToBezierHS OutputVertices 3

tools/clang/test/CodeGenSPIRV/semantic.tess-factor.size-mismatch.ds.hlsl

@@ -0,0 +1,50 @@
+// Run: %dxc -T ds_6_0 -E BezierEvalDS
+
+// Test handling of built-in size mismatch (reading in from the built-ins):
+// The HLSL SV_TessFactor is a float3, but the SPIR-V equivalent is a float4.
+// The HLSL SV_InsideTessFactor is a scalar float, but the SPIR-V equivalent is a float2.
+
+// CHECK: OpDecorate %gl_TessLevelOuter BuiltIn TessLevelOuter
+// CHECK: OpDecorate %gl_TessLevelInner BuiltIn TessLevelInner
+
+// CHECK: %HS_CONSTANT_DATA_OUTPUT = OpTypeStruct %_arr_float_uint_3 %float
+
+// CHECK: %gl_TessLevelOuter = OpVariable %_ptr_Input__arr_float_uint_4 Input
+// CHECK: %gl_TessLevelInner = OpVariable %_ptr_Input__arr_float_uint_2 Input
+
+// CHECK:         [[gl_TessLevelOuter:%\d+]] = OpLoad %_arr_float_uint_4 %gl_TessLevelOuter
+// CHECK-NEXT:   [[gl_TessLevelOuter0:%\d+]] = OpCompositeExtract %float [[gl_TessLevelOuter]] 0
+// CHECK-NEXT:   [[gl_TessLevelOuter1:%\d+]] = OpCompositeExtract %float [[gl_TessLevelOuter]] 1
+// CHECK-NEXT:   [[gl_TessLevelOuter2:%\d+]] = OpCompositeExtract %float [[gl_TessLevelOuter]] 2
+// CHECK-NEXT:   [[tessLevelOuterArr3:%\d+]] = OpCompositeConstruct %_arr_float_uint_3 [[gl_TessLevelOuter0]] [[gl_TessLevelOuter1]] [[gl_TessLevelOuter2]]
+// CHECK-NEXT:    [[gl_TessLevelInner:%\d+]] = OpLoad %_arr_float_uint_2 %gl_TessLevelInner
+// CHECK-NEXT: [[tessLevelOuterScalar:%\d+]] = OpCompositeExtract %float [[gl_TessLevelInner]] 0
+// CHECK-NEXT:                      {{%\d+}} = OpCompositeConstruct %HS_CONSTANT_DATA_OUTPUT [[tessLevelOuterArr3]] [[tessLevelOuterScalar]]
+
+
+struct HS_CONSTANT_DATA_OUTPUT
+{
+  float Edges[3]    : SV_TessFactor;
+  float Inside      : SV_InsideTessFactor;
+};
+
+// Output control point (output of hull shader)
+struct BEZIER_CONTROL_POINT
+{
+  float3 vPosition	: BEZIERPOS;
+};
+
+// The domain shader outputs
+struct DS_OUTPUT
+{
+  float4 vPosition  : SV_POSITION;
+};
+
+[domain("tri")]
+DS_OUTPUT BezierEvalDS( HS_CONSTANT_DATA_OUTPUT input,
+                        float2 UV : SV_DomainLocation,
+                        const OutputPatch<BEZIER_CONTROL_POINT, 3> bezpatch )
+{
+  DS_OUTPUT Output;
+  return Output;
+}

tools/clang/test/CodeGenSPIRV/semantic.tess-factor.size-mismatch.hs.hlsl

@@ -0,0 +1,88 @@
+// Run: %dxc -T hs_6_0 -E SubDToBezierHS
+
+// Test handling of built-in size mismatch (writing out to the built-ins):
+// The HLSL SV_TessFactor is a float3, but the SPIR-V equivalent is a float4.
+// The HLSL SV_InsideTessFactor is a scalar float, but the SPIR-V equivalent is a float2.
+
+
+// CHECK: OpDecorate %gl_TessLevelOuter BuiltIn TessLevelOuter
+// CHECK: OpDecorate %gl_TessLevelInner BuiltIn TessLevelInner
+ 
+// CHECK: %HS_CONSTANT_DATA_OUTPUT = OpTypeStruct %_arr_float_uint_3 %float %_arr_v3float_uint_4 %_arr_v2float_uint_4 %_arr_v3float_uint_4 %_arr_v3float_uint_4 %v4float
+
+// CHECK: %gl_TessLevelOuter = OpVariable %_ptr_Output__arr_float_uint_4 Output
+// CHECK: %gl_TessLevelInner = OpVariable %_ptr_Output__arr_float_uint_2 Output
+
+// CHECK:                      [[pcfRet:%\d+]] = OpFunctionCall %HS_CONSTANT_DATA_OUTPUT %SubDToBezierConstantsHS %param_var_ip %param_var_PatchID
+
+// CHECK-NEXT:       [[pcfRetTessFactor:%\d+]] = OpCompositeExtract %_arr_float_uint_3 [[pcfRet]] 0
+// CHECK-NEXT: [[gl_TessLevelOuter_loc0:%\d+]] = OpAccessChain %_ptr_Output_float %gl_TessLevelOuter %uint_0
+// CHECK-NEXT:      [[pcfRetTessFactor0:%\d+]] = OpCompositeExtract %float [[pcfRetTessFactor]] 0
+// CHECK-NEXT:                                   OpStore [[gl_TessLevelOuter_loc0]] [[pcfRetTessFactor0]]
+
+// CHECK-NEXT: [[gl_TessLevelOuter_loc1:%\d+]] = OpAccessChain %_ptr_Output_float %gl_TessLevelOuter %uint_1
+// CHECK-NEXT:      [[pcfRetTessFactor1:%\d+]] = OpCompositeExtract %float [[pcfRetTessFactor]] 1
+// CHECK-NEXT:                                   OpStore [[gl_TessLevelOuter_loc1]] [[pcfRetTessFactor1]]
+
+// CHECK-NEXT: [[gl_TessLevelOuter_loc2:%\d+]] = OpAccessChain %_ptr_Output_float %gl_TessLevelOuter %uint_2
+// CHECK-NEXT:      [[pcfRetTessFactor2:%\d+]] = OpCompositeExtract %float [[pcfRetTessFactor]] 2
+// CHECK-NEXT:                                   OpStore [[gl_TessLevelOuter_loc2]] [[pcfRetTessFactor2]]
+
+
+// CHECK-NEXT: [[pcfRetInsideTessFactor:%\d+]] = OpCompositeExtract %float [[pcfRet]] 1
+// CHECK-NEXT: [[gl_TessLevelInner_loc0:%\d+]] = OpAccessChain %_ptr_Output_float %gl_TessLevelInner %uint_0
+// CHECK-NEXT:                                   OpStore [[gl_TessLevelInner_loc0]] [[pcfRetInsideTessFactor]]
+
+#define MAX_POINTS 3
+
+// Input control point
+struct VS_CONTROL_POINT_OUTPUT
+{
+  float3 vPosition : WORLDPOS;
+  float2 vUV       : TEXCOORD0;
+  float3 vTangent  : TANGENT;
+};
+
+// Output control point
+struct BEZIER_CONTROL_POINT
+{
+  float3 vPosition	: BEZIERPOS;
+};
+
+// Output patch constant data.
+struct HS_CONSTANT_DATA_OUTPUT
+{
+  float Edges[3]        : SV_TessFactor;
+  float Inside          : SV_InsideTessFactor;
+
+  float3 vTangent[4]    : TANGENT;
+  float2 vUV[4]         : TEXCOORD;
+  float3 vTanUCorner[4] : TANUCORNER;
+  float3 vTanVCorner[4] : TANVCORNER;
+  float4 vCWts          : TANWEIGHTS;
+};
+
+// Patch Constant Function
+HS_CONSTANT_DATA_OUTPUT SubDToBezierConstantsHS(InputPatch<VS_CONTROL_POINT_OUTPUT, MAX_POINTS> ip, uint PatchID : SV_PrimitiveID) {
+  HS_CONSTANT_DATA_OUTPUT Output;
+
+  // Must initialize Edges and Inside; otherwise HLSL validation will fail.
+  Output.Edges[0]  = 1.0;
+  Output.Edges[1]  = 2.0;
+  Output.Edges[2]  = 3.0;
+  Output.Inside    = 4.0;
+
+  return Output;
+}
+
+[domain("tri")]
+[partitioning("fractional_odd")]
+[outputtopology("triangle_ccw")]
+[outputcontrolpoints(MAX_POINTS)]
+[patchconstantfunc("SubDToBezierConstantsHS")]
+BEZIER_CONTROL_POINT SubDToBezierHS(InputPatch<VS_CONTROL_POINT_OUTPUT, MAX_POINTS> ip, uint cpid : SV_OutputControlPointID, uint PatchID : SV_PrimitiveID) {
+  VS_CONTROL_POINT_OUTPUT vsOutput;
+  BEZIER_CONTROL_POINT result;
+  result.vPosition = vsOutput.vPosition;
+  return result;
+}

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -396,12 +396,18 @@ TEST_F(FileTest, SemanticDomainLocationDS) {
 TEST_F(FileTest, SemanticTessFactorDS) {
   runFileTest("semantic.tess-factor.ds.hlsl");
 }
+TEST_F(FileTest, SemanticTessFactorSizeMismatchDS) {
+  runFileTest("semantic.tess-factor.size-mismatch.ds.hlsl");
+}
 TEST_F(FileTest, SemanticInsideTessFactorDS) {
   runFileTest("semantic.inside-tess-factor.ds.hlsl");
 }
 TEST_F(FileTest, SemanticTessFactorHS) {
   runFileTest("semantic.tess-factor.hs.hlsl");
 }
+TEST_F(FileTest, SemanticTessFactorSizeMismatchHS) {
+  runFileTest("semantic.tess-factor.size-mismatch.hs.hlsl");
+}
 TEST_F(FileTest, SemanticInsideTessFactorHS) {
   runFileTest("semantic.inside-tess-factor.hs.hlsl");
 }