Browse Source

Fix validation bugs. (#10)

Xiang Li 8 years ago
parent
commit
cb0ba446c1

+ 2 - 1
docs/DXIL.rst

@@ -2241,7 +2241,7 @@ SM.GSOUTPUTVERTEXCOUNTRANGE           GS output vertex count must be [0..%0].  %
 SM.GSTOTALOUTPUTVERTEXDATARANGE       Declared output vertex count (%0) multiplied by the total number of declared scalar components of output data (%1) equals %2.  This value cannot be greater than %3
 SM.GSVALIDINPUTPRIMITIVE              GS input primitive unrecognized
 SM.GSVALIDOUTPUTPRIMITIVETOPOLOGY     GS output primitive topology unrecognized
-SM.HSINPUTCONTROLPOINTCOUNTRANGE      HS input control point count must be [1..%0].  %1 specified
+SM.HSINPUTCONTROLPOINTCOUNTRANGE      HS input control point count must be [0..%0].  %1 specified
 SM.HULLPASSTHRUCONTROLPOINTCOUNTMATCH For pass thru hull shader, input control point count must match output control point count
 SM.INSIDETESSFACTORSIZEMATCHDOMAIN    InsideTessFactor rows, columns (%0, %1) invalid for domain %2.  Expected %3 rows and 1 column.
 SM.INVALIDRESOURCECOMPTYPE            Invalid resource return type
@@ -2277,6 +2277,7 @@ SM.THREADGROUPCHANNELRANGE            Declared Thread Group %0 size %1 outside v
 SM.TRIOUTPUTPRIMITIVEMISMATCH         Hull Shader declared with Tri Domain must specify output primitive point, triangle_cw or triangle_ccw. Line output is not compatible with the Tri domain
 SM.UNDEFINEDOUTPUT                    Not all elements of output %0 were written
 SM.VALIDDOMAIN                        Invalid Tessellator Domain specified. Must be isoline, tri or quad
+SM.ZEROHSINPUTCONTROLPOINTWITHINPUT   When HS input control point count is 0, no input signature should exist
 TYPES.DEFINED                         Type must be defined based on DXIL primitives
 TYPES.I8                              I8 can only used as immediate value for intrinsic
 TYPES.INTWIDTH                        Int type must be of valid width

+ 2 - 1
include/dxc/HLSL/DxilValidation.h

@@ -178,7 +178,7 @@ enum class ValidationRule : unsigned {
   SmGSTotalOutputVertexDataRange, // Declared output vertex count (%0) multiplied by the total number of declared scalar components of output data (%1) equals %2.  This value cannot be greater than %3
   SmGSValidInputPrimitive, // GS input primitive unrecognized
   SmGSValidOutputPrimitiveTopology, // GS output primitive topology unrecognized
-  SmHSInputControlPointCountRange, // HS input control point count must be [1..%0].  %1 specified
+  SmHSInputControlPointCountRange, // HS input control point count must be [0..%0].  %1 specified
   SmHullPassThruControlPointCountMatch, // For pass thru hull shader, input control point count must match output control point count
   SmInsideTessFactorSizeMatchDomain, // InsideTessFactor rows, columns (%0, %1) invalid for domain %2.  Expected %3 rows and 1 column.
   SmInvalidResourceCompType, // Invalid resource return type
@@ -214,6 +214,7 @@ enum class ValidationRule : unsigned {
   SmTriOutputPrimitiveMismatch, // Hull Shader declared with Tri Domain must specify output primitive point, triangle_cw or triangle_ccw. Line output is not compatible with the Tri domain
   SmUndefinedOutput, // Not all elements of output %0 were written
   SmValidDomain, // Invalid Tessellator Domain specified. Must be isoline, tri or quad
+  SmZeroHSInputControlPointWithInput, // When HS input control point count is 0, no input signature should exist
 
   // Type system
   TypesDefined, // Type must be defined based on DXIL primitives

+ 8 - 3
lib/HLSL/DxilValidation.cpp

@@ -205,7 +205,8 @@ const char *hlsl::GetValidationRuleText(ValidationRule value) {
     case hlsl::ValidationRule::SmGSOutputVertexCountRange: return "GS output vertex count must be [0..%0].  %1 specified";
     case hlsl::ValidationRule::SmGSInstanceCountRange: return "GS instance count must be [1..%0].  %1 specified";
     case hlsl::ValidationRule::SmDSInputControlPointCountRange: return "DS input control point count must be [0..%0].  %1 specified";
-    case hlsl::ValidationRule::SmHSInputControlPointCountRange: return "HS input control point count must be [1..%0].  %1 specified";
+    case hlsl::ValidationRule::SmHSInputControlPointCountRange: return "HS input control point count must be [0..%0].  %1 specified";
+    case hlsl::ValidationRule::SmZeroHSInputControlPointWithInput: return "When HS input control point count is 0, no input signature should exist";
     case hlsl::ValidationRule::SmOutputControlPointCountRange: return "output control point count must be [0..%0].  %1 specified";
     case hlsl::ValidationRule::SmGSValidInputPrimitive: return "GS input primitive unrecognized";
     case hlsl::ValidationRule::SmGSValidOutputPrimitiveTopology: return "GS output primitive topology unrecognized";
@@ -3732,8 +3733,12 @@ static void ValidateShaderState(ValidationContext &ValCtx) {
     if (domain >= DXIL::TessellatorDomain::LastEntry)
       domain = DXIL::TessellatorDomain::Undefined;
     unsigned inputControlPointCount = M.GetInputControlPointCount();
-    if (inputControlPointCount < 1 ||
-        inputControlPointCount > DXIL::kMaxIAPatchControlPointCount) {
+    if (inputControlPointCount == 0) {
+      if (!M.GetInputSignature().GetElements().empty()) {
+        ValCtx.EmitError(
+            ValidationRule::SmZeroHSInputControlPointWithInput);
+      }
+    } else if (inputControlPointCount > DXIL::kMaxIAPatchControlPointCount) {
       ValCtx.EmitFormatError(
           ValidationRule::SmHSInputControlPointCountRange,
           {std::to_string(DXIL::kMaxIAPatchControlPointCount).c_str(),

+ 1 - 1
tools/clang/test/CodeGenHLSL/UndefValue.hlsl

@@ -1,6 +1,6 @@
 // RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
 
-// CHECK: @main
+// CHECK: Instructions should not read uninitialized value
 
 float main(snorm float b : B) : SV_DEPTH
 {

+ 11 - 9
tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -69,6 +69,7 @@ public:
   TEST_METHOD(WhenMultipleModulesThenFail);
   TEST_METHOD(WhenUnexpectedEOFThenFail);
   TEST_METHOD(WhenUnknownBlocksThenFail);
+  TEST_METHOD(WhenZeroInputPatchCountWithInputThenFail);
 
   TEST_METHOD(LoadOutputControlPointNotInPatchConstantFunction);
   TEST_METHOD(StorePatchControlNotInPatchConstantFunction);
@@ -549,6 +550,14 @@ TEST_F(ValidationTest, WhenUnknownBlocksThenFail) {
   CheckValidationMsgs(blob, _countof(blob), "Unrecognized block found");
 }
 
+TEST_F(ValidationTest, WhenZeroInputPatchCountWithInputThenFail) {
+	RewriteAssemblyCheckMsg(
+		L"..\\CodeGenHLSL\\SimpleHs1.hlsl", "hs_6_0",
+		"void ()* @\"\\01?HSPerPatchFunc@@YA?AUHSPerPatchData@@V?$InputPatch@UPSSceneIn@@$02@@@Z.flat\", i32 3, i32 3",
+		"void ()* @\"\\01?HSPerPatchFunc@@YA?AUHSPerPatchData@@V?$InputPatch@UPSSceneIn@@$02@@@Z.flat\", i32 0, i32 3",
+		"When HS input control point count is 0, no input signature should exist");
+}
+
 TEST_F(ValidationTest, WhenInstrDisallowedThenFail) {
   RewriteAssemblyCheckMsg(
       L"..\\CodeGenHLSL\\abs2.hlsl", "ps_6_0",
@@ -678,7 +687,7 @@ TEST_F(ValidationTest, HsAttributeFail) {
       },
       {"i32 36, i32 36, i32 0, i32 0, i32 0, float 6.500000e+01"
       },
-      {"HS input control point count must be [1..32].  36 specified",
+      {"HS input control point count must be [0..32].  36 specified",
        "Invalid Tessellator Domain specified. Must be isoline, tri or quad",
        "Invalid Tessellator Partitioning specified",
        "Invalid Tessellator Output Primitive specified",
@@ -900,14 +909,7 @@ TEST_F(ValidationTest, UavBarrierFail) {
        "sync in a non-Compute Shader must only sync UAV (sync_uglobal)"});
 }
 TEST_F(ValidationTest, UndefValueFail) {
-  RewriteAssemblyCheckMsg(
-      L"..\\CodeGenHLSL\\UndefValue.hlsl", "ps_6_0",
-      {"fadd fast float %([0-9]+)"
-      },
-      {"fadd fast float undef"
-      },
-      {"Instructions should not read uninitialized value"},
-      /*bRegex*/ true);
+  TestCheck(L"..\\CodeGenHLSL\\UndefValue.hlsl");
 }
 TEST_F(ValidationTest, UpdateCounterFail) {
   RewriteAssemblyCheckMsg(

+ 2 - 1
utils/hct/hctdb.py

@@ -1613,7 +1613,8 @@ class db_dxil(object):
         self.add_valrule("Sm.GSOutputVertexCountRange", "GS output vertex count must be [0..%0].  %1 specified")
         self.add_valrule("Sm.GSInstanceCountRange", "GS instance count must be [1..%0].  %1 specified")
         self.add_valrule("Sm.DSInputControlPointCountRange", "DS input control point count must be [0..%0].  %1 specified")
-        self.add_valrule("Sm.HSInputControlPointCountRange", "HS input control point count must be [1..%0].  %1 specified")
+        self.add_valrule("Sm.HSInputControlPointCountRange", "HS input control point count must be [0..%0].  %1 specified")
+        self.add_valrule("Sm.ZeroHSInputControlPointWithInput", "When HS input control point count is 0, no input signature should exist")
         self.add_valrule("Sm.OutputControlPointCountRange", "output control point count must be [0..%0].  %1 specified")
         self.add_valrule("Sm.GSValidInputPrimitive", "GS input primitive unrecognized")
         self.add_valrule("Sm.GSValidOutputPrimitiveTopology", "GS output primitive topology unrecognized")