ViewIDPipelineValidation fixes (#339)

- update DxilPipelineStateValidation with MaxVertexCount
- Add option to ignore indexing constraint in DxilSignatureAllocator
- Fix issues with ViewIDPipelineValidation
- Detect ViewID dependent tessfactors
- Add mode for checking expanded merged input only
- Treat expanded Clip/Cull as Arbitrary

include/dxc/HLSL/DxilPipelineStateValidation.h

@@ -71,6 +71,10 @@ struct PSVRuntimeInfo1 : public PSVRuntimeInfo0
 {
   uint8_t ShaderStage;              // PSVShaderKind
   uint8_t UsesViewID;
+  union {
+    uint16_t MaxVertexCount;          // MaxVertexCount for GS only (max 1024)
+    uint8_t SigPatchConstantVectors;  // Output for HS; Input for DS
+  };
 
   // PSVSignatureElement counts
   uint8_t SigInputElements;
@@ -79,9 +83,7 @@ struct PSVRuntimeInfo1 : public PSVRuntimeInfo0
 
   // Number of packed vectors per signature
   uint8_t SigInputVectors;
-  uint8_t SigPatchConstantVectors;  // Output for HS Input for DS
   uint8_t SigOutputVectors[4];      // Array for GS Stream Out Index
-  uint8_t Reserved;
 };
 
 enum class PSVResourceType
@@ -463,7 +465,7 @@ public:
           if (!IsGS())
             break;
         }
-        if (m_pPSVRuntimeInfo1->ShaderStage == (uint8_t)PSVShaderKind::Hull && m_pPSVRuntimeInfo1->SigPatchConstantVectors) {
+        if (IsHS() && m_pPSVRuntimeInfo1->SigPatchConstantVectors) {
           minsize += sizeof(uint32_t) * PSVComputeMaskDwordsFromVectors(m_pPSVRuntimeInfo1->SigPatchConstantVectors);
           if (!(size >= minsize)) return false;
           m_pViewIDPCOutputMask = (uint32_t*)pCurBits;
@@ -599,7 +601,9 @@ public:
       m_pPSVRuntimeInfo1->SigPatchConstantElements = initInfo.SigPatchConstantElements;
       m_pPSVRuntimeInfo1->SigInputVectors = initInfo.SigInputVectors;
       memcpy(m_pPSVRuntimeInfo1->SigOutputVectors, initInfo.SigOutputVectors, 4);
-      m_pPSVRuntimeInfo1->SigPatchConstantVectors = initInfo.SigPatchConstantVectors;
+      if (IsHS() || IsDS()) {
+        m_pPSVRuntimeInfo1->SigPatchConstantVectors = initInfo.SigPatchConstantVectors;
+      }
 
       // Note: if original size was unaligned, padding has already been zero initialized
       m_StringTable.Size = PSVALIGN4(initInfo.StringTable.Size);
@@ -795,6 +799,7 @@ namespace hlsl {
   public:
     enum class Result {
       Success = 0,
+      SuccessWithViewIDDependentTessFactor,
       InsufficientSpace,
       InsufficientPCSpace,
       MismatchedSignatures,
@@ -806,6 +811,7 @@ namespace hlsl {
     virtual ~ViewIDValidator() {}
     virtual Result ValidateStage(const DxilPipelineStateValidation &PSV,
                                  bool bFinalStage,
+                                 bool bExpandInputOnly,
                                  unsigned &mismatchElementId) = 0;
   };
 

include/dxc/HLSL/DxilSignatureAllocator.h

@@ -115,10 +115,11 @@ public:
     void PlaceElement(uint8_t flags, uint8_t indexFlags, DXIL::InterpolationMode interp, unsigned col, unsigned width);
   };
 
-  std::vector<PackedRegister> Registers;
-
   DxilSignatureAllocator(unsigned numRegisters);
 
+  bool GetIgnoreIndexing() const { return m_bIgnoreIndexing; }
+  void SetIgnoreIndexing(bool ignoreIndexing) { m_bIgnoreIndexing  = ignoreIndexing; }
+
   ConflictType DetectRowConflict(const PackElement *SE, unsigned row);
   ConflictType DetectColConflict(const PackElement *SE, unsigned row, unsigned col);
   void PlaceElement(const PackElement *SE, unsigned row, unsigned col);
@@ -134,6 +135,9 @@ public:
   // Pack in a prefix-stable way - appended elements do not affect positions of prior elements.
   unsigned PackPrefixStable(std::vector<PackElement*> elements, unsigned startRow, unsigned numRows);
 
+protected:
+  std::vector<PackedRegister> m_Registers;
+  bool m_bIgnoreIndexing;
 };
 
 

include/dxc/HLSL/DxilSignatureAllocator.inl

@@ -136,19 +136,21 @@ void DxilSignatureAllocator::PackedRegister::PlaceElement(uint8_t flags, uint8_t
   }
 }
 
-DxilSignatureAllocator::DxilSignatureAllocator(unsigned numRegisters) {
-  Registers.resize(numRegisters);
+DxilSignatureAllocator::DxilSignatureAllocator(unsigned numRegisters)
+  : m_bIgnoreIndexing(false) {
+  m_Registers.resize(numRegisters);
 }
 
 DxilSignatureAllocator::ConflictType DxilSignatureAllocator::DetectRowConflict(const PackElement *SE, unsigned row) {
   unsigned rows = SE->GetRows();
-  if (rows + row > Registers.size())
+  if (rows + row > m_Registers.size())
     return kConflictFit;
   unsigned cols = SE->GetCols();
   DXIL::InterpolationMode interp = SE->GetInterpolationMode();
   uint8_t flags = GetElementFlags(SE);
   for (unsigned i = 0; i < rows; ++i) {
-    ConflictType conflict = Registers[row + i].DetectRowConflict(flags, GetIndexFlags(i, rows), interp, cols);
+    uint8_t indexFlags = m_bIgnoreIndexing ? 0 : GetIndexFlags(i, rows);
+    ConflictType conflict = m_Registers[row + i].DetectRowConflict(flags, indexFlags, interp, cols);
     if (conflict)
       return conflict;
   }
@@ -160,7 +162,7 @@ DxilSignatureAllocator::ConflictType DxilSignatureAllocator::DetectColConflict(c
   unsigned cols = SE->GetCols();
   uint8_t flags = GetElementFlags(SE);
   for (unsigned i = 0; i < rows; ++i) {
-    ConflictType conflict = Registers[row + i].DetectColConflict(flags, col, cols);
+    ConflictType conflict = m_Registers[row + i].DetectColConflict(flags, col, cols);
     if (conflict)
       return conflict;
   }
@@ -174,7 +176,8 @@ void DxilSignatureAllocator::PlaceElement(const PackElement *SE, unsigned row, u
   DXIL::InterpolationMode interp = SE->GetInterpolationMode();
   uint8_t flags = GetElementFlags(SE);
   for (unsigned i = 0; i < rows; ++i) {
-    Registers[row + i].PlaceElement(flags, GetIndexFlags(i, rows), interp, col, cols);
+    uint8_t indexFlags = m_bIgnoreIndexing ? 0 : GetIndexFlags(i, rows);
+    m_Registers[row + i].PlaceElement(flags, indexFlags, interp, col, cols);
   }
 }
 
@@ -191,14 +194,14 @@ int cmp(T a, T b) {
 }
 int CmpElements(const DxilSignatureAllocator::PackElement* left, const DxilSignatureAllocator::PackElement* right) {
   unsigned result;
-  if (result = cmp((unsigned)left->GetInterpolationMode(), (unsigned)right->GetInterpolationMode()))
-    return result;
-  if (result = -cmp(left->GetRows(), right->GetRows()))
-    return result;
-  if (result = -cmp(left->GetCols(), right->GetCols()))
-    return result;
-  if (result = cmp(left->GetID(), right->GetID()))
-    return result;
+  result = cmp((unsigned)left->GetInterpolationMode(), (unsigned)right->GetInterpolationMode());
+  if (result) return result;
+  result = -cmp(left->GetRows(), right->GetRows());
+  if (result) return result;
+  result = -cmp(left->GetCols(), right->GetCols());
+  if (result) return result;
+  result = cmp(left->GetID(), right->GetID());
+  if (result) return result;
   return 0;
 }
 

include/dxc/HLSL/ViewIDPipelineValidation.inl

@@ -37,6 +37,10 @@ struct ComponentMask : public PSVComponentMask {
     }
     return *this;
   }
+  ComponentMask &operator=(const ComponentMask &other) {
+    *this = (const PSVComponentMask &)other;
+    return *this;
+  }
   ComponentMask &operator|=(const PSVComponentMask &other) {
     NumVectors = std::max(NumVectors, other.NumVectors);
     PSVComponentMask::operator|=(other);
@@ -94,13 +98,19 @@ static void AddViewIDElements(ElementVec &outElements,
           if (adding) {
             uint32_t componentIndex = (E.GetStartRow() + row) * 4 + E.GetStartCol() + col;
             DxilSignatureAllocator::DummyElement NE(numElements);
-            NE.rows = bDynIndex ? E.GetRows() : 1;
-            if (mask.Get(componentIndex)) {
-              NE.rows *= viewIDCount;
-            }
             NE.kind = E.kind;
             NE.interpolation = E.interpolation;
             NE.interpretation = E.interpretation;
+            NE.rows = bDynIndex ? E.GetRows() : 1;
+            for (uint32_t indexedRow = 0; indexedRow < NE.rows; indexedRow++) {
+              if (mask.Get(componentIndex + (4 * indexedRow))) {
+                if (E.GetKind() == DXIL::SemanticKind::ClipDistance || E.GetKind() == DXIL::SemanticKind::CullDistance) {
+                  NE.kind = DXIL::SemanticKind::Arbitrary;
+                }
+                NE.rows *= viewIDCount;
+                break;
+              }
+            }
             outElements.push_back(NE);
           }
           numElements++;
@@ -108,7 +118,7 @@ static void AddViewIDElements(ElementVec &outElements,
       }
     }
     if (!adding)
-      outElements.resize(numElements);
+      outElements.reserve(numElements);
   }
 }
 
@@ -118,6 +128,7 @@ static bool CheckFit(ElementVec &elements) {
   for (auto &E : elements)
     packElements.push_back(&E);
   DxilSignatureAllocator alloc(32);
+  alloc.SetIgnoreIndexing(true);
   alloc.PackOptimized(packElements, 0, 32);
   for (auto &E : elements) {
     if (!E.IsAllocated())
@@ -126,6 +137,13 @@ static bool CheckFit(ElementVec &elements) {
   return true;
 }
 
+static bool CheckMaxVertexCount(const ElementVec &elements, unsigned maxVertexCount) {
+  unsigned numComponents = 0;
+  for (auto &E : elements)
+    numComponents += E.GetRows() * E.GetCols();
+  return numComponents <= 1024 && maxVertexCount <= 1024 && numComponents * maxVertexCount <= 1024;
+}
+
 static bool MergeElements(const ElementVec &priorElements,
                           ElementVec &inputElements,
                           uint32_t &numVectors,
@@ -182,6 +200,21 @@ static void PropagateMask(const ComponentMask &priorMask,
   }
 }
 
+bool DetectViewIDDependentTessFactor(const ElementVec &pcElements, ComponentMask &mask) {
+  for (auto &E : pcElements) {
+    if (E.GetKind() == DXIL::SemanticKind::TessFactor || E.GetKind() == DXIL::SemanticKind::InsideTessFactor) {
+      for (unsigned row = 0; row < E.GetRows(); row++) {
+        for (unsigned col = 0; col < E.GetCols(); col++) {
+          uint32_t componentIndex = (E.GetStartRow() + row) * 4 + E.GetStartCol() + col;
+          if (mask.Get(componentIndex))
+            return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
 class ViewIDValidator_impl : public hlsl::ViewIDValidator {
   ComponentMask m_PriorOutputMask;
   ComponentMask m_PriorPCMask;
@@ -190,6 +223,13 @@ class ViewIDValidator_impl : public hlsl::ViewIDValidator {
   unsigned m_ViewIDCount;
   unsigned m_GSRastStreamIndex;
 
+  void ClearPriorState() {
+    m_PriorOutputMask = ComponentMask();
+    m_PriorPCMask = ComponentMask();
+    m_PriorOutputSignature.clear();
+    m_PriorPCSignature.clear();
+  }
+
 public:
   ViewIDValidator_impl(unsigned viewIDCount, unsigned gsRastStreamIndex)
     : m_PriorOutputMask(),
@@ -199,6 +239,7 @@ public:
   virtual ~ViewIDValidator_impl() {}
   __override Result ValidateStage(const DxilPipelineStateValidation &PSV,
                                   bool bFinalStage,
+                                  bool bExpandInputOnly,
                                   unsigned &mismatchElementId) {
     if (!PSV.GetPSVRuntimeInfo0())
       return Result::InvalidPSV;
@@ -207,6 +248,9 @@ public:
 
     switch (PSV.GetShaderKind()) {
     case PSVShaderKind::Vertex: {
+      if (bExpandInputOnly)
+        return Result::InvalidUsage;
+
       // Initialize mask with direct ViewID dependent outputs
       ComponentMask mask(PSV.GetViewIDOutputMask(0));
 
@@ -239,14 +283,6 @@ public:
                     [&](unsigned i) -> PSVSignatureElement {
                       return PSV.GetSignatureElement(PSV.GetInputElement0(i));
                     });
-      CopyElements( outSig, DXIL::SigPointKind::HSCPOut, PSV.GetSigOutputElements(), 0,
-                    [&](unsigned i) -> PSVSignatureElement {
-                      return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
-                    });
-      CopyElements( pcSig, DXIL::SigPointKind::PCOut, PSV.GetSigPatchConstantElements(), 0,
-                    [&](unsigned i) -> PSVSignatureElement {
-                      return PSV.GetSignatureElement(PSV.GetPatchConstantElement0(i));
-                    });
 
       // Merge prior and input signatures, update prior mask size if necessary
       if (!MergeElements(m_PriorOutputSignature, inSig, m_PriorOutputMask.NumVectors, mismatchElementId))
@@ -260,6 +296,20 @@ public:
       if (!CheckFit(viewIDSig))
         return Result::InsufficientSpace;
 
+      if (bExpandInputOnly) {
+        ClearPriorState();
+        return Result::Success;
+      }
+
+      CopyElements(outSig, DXIL::SigPointKind::HSCPOut, PSV.GetSigOutputElements(), 0,
+        [&](unsigned i) -> PSVSignatureElement {
+        return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
+      });
+      CopyElements(pcSig, DXIL::SigPointKind::PCOut, PSV.GetSigPatchConstantElements(), 0,
+        [&](unsigned i) -> PSVSignatureElement {
+        return PSV.GetSignatureElement(PSV.GetPatchConstantElement0(i));
+      });
+
       // Propagate prior mask through input-output dependencies
       if (PSV.GetInputToOutputTable(0).IsValid()) {
         PropagateMask(m_PriorOutputMask, inSig, outputMask,
@@ -278,6 +328,10 @@ public:
       m_PriorOutputSignature = std::move(outSig);
       m_PriorPCSignature = std::move(pcSig);
 
+      if (DetectViewIDDependentTessFactor(pcSig, pcMask)) {
+        return Result::SuccessWithViewIDDependentTessFactor;
+      }
+
       break;
     }
     case PSVShaderKind::Domain: {
@@ -294,10 +348,6 @@ public:
                     [&](unsigned i) -> PSVSignatureElement {
                       return PSV.GetSignatureElement(PSV.GetPatchConstantElement0(i));
                     });
-      CopyElements( outSig, DXIL::SigPointKind::DSOut, PSV.GetSigOutputElements(), 0,
-                    [&](unsigned i) -> PSVSignatureElement {
-                      return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
-                    });
 
       // Merge prior and input signatures, update prior mask size if necessary
       if (!MergeElements(m_PriorOutputSignature, inSig, m_PriorOutputMask.NumVectors, mismatchElementId))
@@ -325,6 +375,16 @@ public:
           return Result::InsufficientPCSpace;
       }
 
+      if (bExpandInputOnly) {
+        ClearPriorState();
+        return Result::Success;
+      }
+
+      CopyElements(outSig, DXIL::SigPointKind::DSOut, PSV.GetSigOutputElements(), 0,
+        [&](unsigned i) -> PSVSignatureElement {
+        return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
+      });
+
       // Propagate prior mask through input-output dependencies
       if (PSV.GetInputToOutputTable(0).IsValid()) {
         PropagateMask(m_PriorOutputMask, inSig, mask,
@@ -346,19 +406,12 @@ public:
       break;
     }
     case PSVShaderKind::Geometry: {
-      // Initialize mask with direct ViewID dependent outputs
-      ComponentMask mask(PSV.GetViewIDOutputMask(m_GSRastStreamIndex));
-
       // capture signatures
-      ElementVec inSig, outSig;
+      ElementVec inSig, outSig[4];
       CopyElements( inSig, DXIL::SigPointKind::GSVIn, PSV.GetSigInputElements(), 0,
                     [&](unsigned i) -> PSVSignatureElement {
                       return PSV.GetSignatureElement(PSV.GetInputElement0(i));
                     });
-      CopyElements( outSig, DXIL::SigPointKind::GSOut, PSV.GetSigOutputElements(), m_GSRastStreamIndex,
-                    [&](unsigned i) -> PSVSignatureElement {
-                      return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
-                    });
 
       // Merge prior and input signatures, update prior mask size if necessary
       if (!MergeElements(m_PriorOutputSignature, inSig, m_PriorOutputMask.NumVectors, mismatchElementId))
@@ -372,19 +425,52 @@ public:
       if (!CheckFit(viewIDSig))
         return Result::InsufficientSpace;
 
-      // Propagate prior mask through input-output dependencies
-      if (PSV.GetInputToOutputTable(0).IsValid()) {
-        PropagateMask(m_PriorOutputMask, inSig, mask,
-                      [&](unsigned i) -> PSVComponentMask { return PSV.GetInputToOutputTable(m_GSRastStreamIndex).GetMaskForInput(i); });
+      if (bExpandInputOnly) {
+        ClearPriorState();
+        return Result::Success;
       }
 
-      // Copy mask to prior mask
-      m_PriorOutputMask = mask;
+      for (unsigned streamIndex = 0; streamIndex < 4; streamIndex++) {
+        // Initialize mask with direct ViewID dependent outputs
+        ComponentMask mask(PSV.GetViewIDOutputMask(streamIndex));
 
-      // Capture output signature for next stage
-      m_PriorOutputSignature = std::move(outSig);
+        CopyElements( outSig[streamIndex], DXIL::SigPointKind::GSOut, PSV.GetSigOutputElements(), streamIndex,
+                      [&](unsigned i) -> PSVSignatureElement {
+                        return PSV.GetSignatureElement(PSV.GetOutputElement0(i));
+                      });
 
-      break;
+        if (!outSig[streamIndex].empty()) {
+          // Propagate prior mask through input-output dependencies
+          if (PSV.GetInputToOutputTable(streamIndex).IsValid()) {
+            PropagateMask(m_PriorOutputMask, inSig, mask,
+              [&](unsigned i) -> PSVComponentMask { return PSV.GetInputToOutputTable(streamIndex).GetMaskForInput(i); });
+          }
+
+          // Create new version with ViewID elements from prior signature
+          ElementVec viewIDSig;
+          AddViewIDElements(viewIDSig, outSig[streamIndex], mask, m_ViewIDCount);
+
+          // Verify fit
+          if (!CheckMaxVertexCount(viewIDSig, PSV.GetPSVRuntimeInfo1()->MaxVertexCount))
+            return Result::InsufficientSpace;
+          if (!CheckFit(viewIDSig))
+            return Result::InsufficientSpace;
+        }
+
+        // Capture this mask for the next stage
+        if (m_GSRastStreamIndex == streamIndex)
+          m_PriorOutputMask = mask;
+      }
+
+      if (m_GSRastStreamIndex < 4 && !bFinalStage) {
+        m_PriorOutputSignature = std::move(outSig[m_GSRastStreamIndex]);
+      } else {
+        ClearPriorState();
+        if (!bFinalStage)
+          return Result::InvalidUsage;
+      }
+
+      return Result::Success;
     }
     case PSVShaderKind::Pixel: {
       // capture signatures