Add ViewID slot validation for PSIn (#254)

docs/DXIL.rst

@@ -2941,6 +2941,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.VIEWIDNEEDSSLOT                    ViewID requires compatible space in pixel shader input signature
 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

include/dxc/HLSL/DxilValidation.h

@@ -215,6 +215,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
+  SmViewIDNeedsSlot, // ViewID requires compatible space in pixel shader input signature
   SmZeroHSInputControlPointWithInput, // When HS input control point count is 0, no input signature should exist
 
   // Type system

lib/HLSL/DxilValidation.cpp

@@ -227,6 +227,7 @@ const char *hlsl::GetValidationRuleText(ValidationRule value) {
     case hlsl::ValidationRule::SmCBufferOffsetOverlap: return "CBuffer %0 has offset overlaps at %1";
     case hlsl::ValidationRule::SmCBufferElementOverflow: return "CBuffer %0 size insufficient for element at offset %1";
     case hlsl::ValidationRule::SmOpcodeInInvalidFunction: return "opcode '%0' should only used in '%1'";
+    case hlsl::ValidationRule::SmViewIDNeedsSlot: return "Pixel shader input signature lacks available space for ViewID";
     case hlsl::ValidationRule::UniNoWaveSensitiveGradient: return "Gradient operations are not affected by wave-sensitive data or control flow.";
     case hlsl::ValidationRule::FlowReducible: return "Execution flow must be reducible";
     case hlsl::ValidationRule::FlowNoRecusion: return "Recursion is not permitted";
@@ -348,6 +349,7 @@ struct ValidationContext {
   std::unordered_set<Function *> patchConstFuncCallSet;
   std::unordered_map<unsigned, bool> UavCounterIncMap;
   bool hasOutputPosition[DXIL::kNumOutputStreams];
+  bool hasViewID;
   unsigned OutputPositionMask[DXIL::kNumOutputStreams];
   std::vector<unsigned> outputCols;
   std::vector<unsigned> patchConstCols;
@@ -369,7 +371,8 @@ struct ValidationContext {
             DxilMDHelper::kDxilPreciseAttributeMDName)),
         kLLVMLoopMDKind(llvmModule.getContext().getMDKindID("llvm.loop")),
         DiagPrinter(DiagPrn), LastRuleEmit((ValidationRule)-1),
-        m_bCoverageIn(false), m_bInnerCoverageIn(false) {
+        m_bCoverageIn(false), m_bInnerCoverageIn(false),
+        hasViewID(false) {
     DxilMod.GetDxilVersion(m_DxilMajor, m_DxilMinor);
     for (unsigned i = 0; i < DXIL::kNumOutputStreams; i++) {
       hasOutputPosition[i] = false;
@@ -1845,6 +1848,9 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
   case DXIL::OpCode::InnerCoverage:
     ValCtx.m_bInnerCoverageIn = true;
     break;
+  case DXIL::OpCode::ViewID:
+    ValCtx.hasViewID = true;
+    break;
   default:
     // Skip opcodes don't need special check.
     break;
@@ -3502,6 +3508,18 @@ static void ValidateSignature(ValidationContext &ValCtx, const DxilSignature &S,
       ValCtx.hasOutputPosition[E->GetOutputStream()] = true;
     }
   }
+
+  if (ValCtx.hasViewID && S.IsInput() && ValCtx.DxilMod.GetShaderModel()->GetKind() == DXIL::ShaderKind::Pixel) {
+    // Ensure sufficient space for ViewID:
+    DxilSignatureElement viewID(DXIL::SigPointKind::PSIn);
+    // Don't use SV_ViewID here because it will be rejected by packing (NotInSig).
+    // Instead, use arbitrary with uint32 type and constant interpolation.
+    viewID.Initialize("ViewID", hlsl::CompType::getU32(), DXIL::InterpolationMode::Constant, 1, 1);
+    allocator[0].PackNext(&viewID, 0, 32);
+    if (!viewID.IsAllocated()) {
+      ValCtx.EmitError(ValidationRule::SmViewIDNeedsSlot);
+    }
+  }
 }
 
 static void ValidateNoInterpModeSignature(ValidationContext &ValCtx, const DxilSignature &S) {
@@ -4046,7 +4064,6 @@ ValidateDxilModule(llvm::Module *pModule, llvm::Module *pDebugModule) {
   ValidateGlobalVariables(ValCtx);
 
   ValidateResources(ValCtx);
-  ValidateSignatures(ValCtx);
 
   // Validate control flow and collect function call info.
   // If has recursive call, call info collection will not finish.
@@ -4061,6 +4078,8 @@ ValidateDxilModule(llvm::Module *pModule, llvm::Module *pDebugModule) {
 
   ValidateShaderFlags(ValCtx);
 
+  ValidateSignatures(ValCtx);
+
   if (!pDxilModule->GetShaderModel()->IsGS()) {
     unsigned posMask = ValCtx.OutputPositionMask[0];
     if (posMask != 0xf && ValCtx.hasOutputPosition[0]) {

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -292,6 +292,7 @@ public:
 
   TEST_METHOD(ViewIDInCSFail)
   TEST_METHOD(ViewIDIn60Fail)
+  TEST_METHOD(ViewIDNoSpaceFail)
 
   dxc::DxcDllSupport m_dllSupport;
   bool m_CompilerPreservesBBNames;
@@ -2922,6 +2923,20 @@ float4 main(float3 pos : Position, uint id : SV_PrimitiveID) : SV_Position \
     /*bRegex*/false);
 }
 
+TEST_F(ValidationTest, ViewIDNoSpaceFail) {
+  RewriteAssemblyCheckMsg(" \
+float4 main(uint vid : SV_ViewID, float3 In[31] : INPUT) : SV_Target \
+{ return float4(In[vid], 1); } \
+    ",
+    "ps_6_1",
+    {"!{i32 0, !\"INPUT\", i8 9, i8 0, !([0-9]+), i8 2, i32 31",
+     "!{i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30}"},
+    {"!{i32 0, !\"INPUT\", i8 9, i8 0, !\\1, i8 2, i32 32",
+     "!{i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31}"},
+    "Pixel shader input signature lacks available space for ViewID",
+    /*bRegex*/true);
+}
+
 
 
 // TODO: reject non-zero padding

utils/hct/hctdb.py

@@ -1689,6 +1689,7 @@ class db_dxil(object):
         self.add_valrule_msg("Sm.CBufferOffsetOverlap", "CBuffer offsets must not overlap", "CBuffer %0 has offset overlaps at %1")
         self.add_valrule_msg("Sm.CBufferElementOverflow", "CBuffer elements must not overflow", "CBuffer %0 size insufficient for element at offset %1")
         self.add_valrule_msg("Sm.OpcodeInInvalidFunction", "Invalid DXIL opcode usage like StorePatchConstant in patch constant function", "opcode '%0' should only used in '%1'")
+        self.add_valrule_msg("Sm.ViewIDNeedsSlot", "ViewID requires compatible space in pixel shader input signature", "Pixel shader input signature lacks available space for ViewID")
 
         # fxc relaxed check of gradient check.
         #self.add_valrule("Uni.NoUniInDiv", "TODO - No instruction requiring uniform execution can be present in divergent block")