DxilValidation: add ShaderCompatInfo call graph checking (#6332) (#6358)

ShaderCompatInfo identifies compatibility for functions that could be
called from an entry point.
Currently, this checking detects compatibility problems between entry
points and internal function calls that the validator otherwise misses.

This change adds a check for ShaderCompatInfo, recursing into called
functions looking for a source of conflict when not compatible with the
current entry point properties.
Errors are emitted for the entry point and at one source of each type of
conflict that ShaderCompatInfo detects.
A function is considered the source of a conflict when it has the
conflict but none of the functions called from this function have this
conflict.

Removed early exit for ShaderFlags validation when module is a library,
since these flags should be validated for libraries, and running
CollectShaderFlagsForModule fills in the ShaderCompatInfo data we need
for validation.

Also fixed tests for illegal barrier and derivative use, as well as
replacing the ignored MaxOutputRecords with the correct MaxRecords
attribute.

Fixes #6292.

(cherry picked from commit 696a13a)

Author: tex3d

docs/DXIL.rst

@@ -3216,6 +3216,14 @@ SM.GSVALIDINPUTPRIMITIVE                  GS input primitive unrecognized.
 SM.GSVALIDOUTPUTPRIMITIVETOPOLOGY         GS output primitive topology unrecognized.
 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.INCOMPATIBLECALLINENTRY                Features used in internal function calls must be compatible with entry
+SM.INCOMPATIBLEDERIVINCOMPUTESHADERMODEL  Derivatives in compute-model shaders require shader model 6.6 and above
+SM.INCOMPATIBLEDERIVLAUNCH                Node shaders only support derivatives in broadcasting launch mode
+SM.INCOMPATIBLEOPERATION                  Operations used in entry function must be compatible with shader stage and other properties
+SM.INCOMPATIBLEREQUIRESGROUP              Functions requiring groupshared memory must be called from shaders with a visible group
+SM.INCOMPATIBLESHADERMODEL                Functions may only use features available in the current shader model
+SM.INCOMPATIBLESTAGE                      Functions may only use features available in the entry function's stage
+SM.INCOMPATIBLETHREADGROUPDIM             When derivatives are used in compute-model shaders, the thread group dimensions must be compatible
 SM.INSIDETESSFACTORSIZEMATCHDOMAIN        InsideTessFactor rows, columns (%0, %1) invalid for domain %2.  Expected %3 rows and 1 column.
 SM.INVALIDRESOURCECOMPTYPE                Invalid resource return type.
 SM.INVALIDRESOURCEKIND                    Invalid resources kind.

lib/HLSL/DxilValidation.cpp

@@ -174,6 +174,7 @@ struct ValidationContext {
   const unsigned kLLVMLoopMDKind;
   unsigned m_DxilMajor, m_DxilMinor;
   ModuleSlotTracker slotTracker;
+  std::unique_ptr<CallGraph> pCallGraph;
 
   ValidationContext(Module &llvmModule, Module *DebugModule,
                     DxilModule &dxilModule)
@@ -397,6 +398,12 @@ struct ValidationContext {
 
   EntryStatus &GetEntryStatus(Function *F) { return *entryStatusMap[F]; }
 
+  CallGraph &GetCallGraph() {
+    if (!pCallGraph)
+      pCallGraph = llvm::make_unique<CallGraph>(M);
+    return *pCallGraph.get();
+  }
+
   DxilResourceProperties GetResourceFromVal(Value *resVal);
 
   void EmitGlobalVariableFormatError(GlobalVariable *GV, ValidationRule rule,
@@ -5386,6 +5393,216 @@ static void ValidateEntrySignatures(ValidationContext &ValCtx) {
   }
 }
 
+// CompatibilityChecker is used to identify incompatibilities in an entry
+// function and any functions called by that entry function.
+struct CompatibilityChecker {
+  ValidationContext &ValCtx;
+  Function *EntryFn;
+  const DxilFunctionProps &props;
+  DXIL::ShaderKind shaderKind;
+
+  // These masks identify the potential conflict flags based on the entry
+  // function's shader kind and properties when either UsesDerivatives or
+  // RequiresGroup flags are set in ShaderCompatInfo.
+  uint32_t maskForDeriv = 0;
+  uint32_t maskForGroup = 0;
+
+  enum class ConflictKind : uint32_t {
+    Stage,
+    ShaderModel,
+    DerivLaunch,
+    DerivThreadGroupDim,
+    DerivInComputeShaderModel,
+    RequiresGroup,
+  };
+  enum class ConflictFlags : uint32_t {
+    Stage = 1 << (uint32_t)ConflictKind::Stage,
+    ShaderModel = 1 << (uint32_t)ConflictKind::ShaderModel,
+    DerivLaunch = 1 << (uint32_t)ConflictKind::DerivLaunch,
+    DerivThreadGroupDim = 1 << (uint32_t)ConflictKind::DerivThreadGroupDim,
+    DerivInComputeShaderModel =
+        1 << (uint32_t)ConflictKind::DerivInComputeShaderModel,
+    RequiresGroup = 1 << (uint32_t)ConflictKind::RequiresGroup,
+  };
+
+  CompatibilityChecker(ValidationContext &ValCtx, Function *EntryFn)
+      : ValCtx(ValCtx), EntryFn(EntryFn),
+        props(ValCtx.DxilMod.GetDxilEntryProps(EntryFn).props),
+        shaderKind(props.shaderKind) {
+
+    // Precompute potential incompatibilities based on shader stage, shader kind
+    // and entry attributes. These will turn into full conflicts if the entry
+    // point's shader flags indicate that they use relevant features.
+    if (!ValCtx.DxilMod.GetShaderModel()->IsSM66Plus() &&
+        (shaderKind == DXIL::ShaderKind::Mesh ||
+         shaderKind == DXIL::ShaderKind::Amplification ||
+         shaderKind == DXIL::ShaderKind::Compute)) {
+      maskForDeriv |=
+          static_cast<uint32_t>(ConflictFlags::DerivInComputeShaderModel);
+    } else if (shaderKind == DXIL::ShaderKind::Node) {
+      // Only broadcasting launch supports derivatives.
+      if (props.Node.LaunchType != DXIL::NodeLaunchType::Broadcasting)
+        maskForDeriv |= static_cast<uint32_t>(ConflictFlags::DerivLaunch);
+      // Thread launch node has no group.
+      if (props.Node.LaunchType == DXIL::NodeLaunchType::Thread)
+        maskForGroup |= static_cast<uint32_t>(ConflictFlags::RequiresGroup);
+    }
+
+    if (shaderKind == DXIL::ShaderKind::Mesh ||
+        shaderKind == DXIL::ShaderKind::Amplification ||
+        shaderKind == DXIL::ShaderKind::Compute ||
+        shaderKind == DXIL::ShaderKind::Node) {
+      // All compute-like stages
+      // Thread dimensions must be either 1D and X is multiple of 4, or 2D
+      // and X and Y must be multiples of 2.
+      if (props.numThreads[1] == 1 && props.numThreads[2] == 1) {
+        if ((props.numThreads[0] & 0x3) != 0)
+          maskForDeriv |=
+              static_cast<uint32_t>(ConflictFlags::DerivThreadGroupDim);
+      } else if ((props.numThreads[0] & 0x1) || (props.numThreads[1] & 0x1))
+        maskForDeriv |=
+            static_cast<uint32_t>(ConflictFlags::DerivThreadGroupDim);
+    } else {
+      // other stages have no group
+      maskForGroup |= static_cast<uint32_t>(ConflictFlags::RequiresGroup);
+    }
+  }
+
+  uint32_t
+  IdentifyConflict(const DxilModule::ShaderCompatInfo &compatInfo) const {
+    uint32_t conflictMask = 0;
+
+    // Compatibility check said this shader kind is not compatible.
+    if (0 == ((1 << (uint32_t)shaderKind) & compatInfo.mask))
+      conflictMask |= (uint32_t)ConflictFlags::Stage;
+
+    // Compatibility check said this shader model is not compatible.
+    if (DXIL::CompareVersions(ValCtx.DxilMod.GetShaderModel()->GetMajor(),
+                              ValCtx.DxilMod.GetShaderModel()->GetMinor(),
+                              compatInfo.minMajor, compatInfo.minMinor) < 0)
+      conflictMask |= (uint32_t)ConflictFlags::ShaderModel;
+
+    if (compatInfo.shaderFlags.GetUsesDerivatives())
+      conflictMask |= maskForDeriv;
+
+    if (compatInfo.shaderFlags.GetRequiresGroup())
+      conflictMask |= maskForGroup;
+
+    return conflictMask;
+  }
+
+  void Diagnose(Function *F, uint32_t conflictMask, ConflictKind conflict,
+                ValidationRule rule, ArrayRef<StringRef> args = {}) {
+    if (conflictMask & (1 << (unsigned)conflict))
+      ValCtx.EmitFnFormatError(F, rule, args);
+  }
+
+  void DiagnoseConflicts(Function *F, uint32_t conflictMask) {
+    // Emit a diagnostic indicating that either the entry function or a function
+    // called by the entry function contains a disallowed operation.
+    if (F == EntryFn)
+      ValCtx.EmitFnError(EntryFn, ValidationRule::SmIncompatibleOperation);
+    else
+      ValCtx.EmitFnError(EntryFn, ValidationRule::SmIncompatibleCallInEntry);
+
+    // Emit diagnostics for each conflict found in this function.
+    Diagnose(F, conflictMask, ConflictKind::Stage,
+             ValidationRule::SmIncompatibleStage,
+             {ShaderModel::GetKindName(props.shaderKind)});
+    Diagnose(F, conflictMask, ConflictKind::ShaderModel,
+             ValidationRule::SmIncompatibleShaderModel);
+    Diagnose(F, conflictMask, ConflictKind::DerivLaunch,
+             ValidationRule::SmIncompatibleDerivLaunch,
+             {GetLaunchTypeStr(props.Node.LaunchType)});
+    Diagnose(F, conflictMask, ConflictKind::DerivThreadGroupDim,
+             ValidationRule::SmIncompatibleThreadGroupDim,
+             {std::to_string(props.numThreads[0]),
+              std::to_string(props.numThreads[1]),
+              std::to_string(props.numThreads[2])});
+    Diagnose(F, conflictMask, ConflictKind::DerivInComputeShaderModel,
+             ValidationRule::SmIncompatibleDerivInComputeShaderModel);
+    Diagnose(F, conflictMask, ConflictKind::RequiresGroup,
+             ValidationRule::SmIncompatibleRequiresGroup);
+  }
+
+  // Visit function and all functions called by it.
+  // Emit diagnostics for incompatibilities found in a function when no
+  // functions called by that function introduced the conflict.
+  // In those cases, the called functions themselves will emit the diagnostic.
+  // Return conflict mask for this function.
+  uint32_t Visit(Function *F, uint32_t &remainingMask,
+                 llvm::SmallPtrSet<Function *, 8> &visited, CallGraph &CG) {
+    // Recursive check looks for where a conflict is found and not present
+    // in functions called by the current function.
+    // - When a source is found, emit diagnostics and clear the conflict
+    // flags introduced by this function from the working mask so we don't
+    // report this conflict again.
+    // - When the remainingMask is 0, we are done.
+
+    if (remainingMask == 0)
+      return 0; // Nothing left to search for.
+    if (!visited.insert(F).second)
+      return 0; // Already visited.
+
+    const DxilModule::ShaderCompatInfo *compatInfo =
+        ValCtx.DxilMod.GetCompatInfoForFunction(F);
+    DXASSERT(compatInfo, "otherwise, compat info not computed in module");
+    if (!compatInfo)
+      return 0;
+    uint32_t maskForThisFunction = IdentifyConflict(*compatInfo);
+
+    uint32_t maskForCalls = 0;
+    if (CallGraphNode *CGNode = CG[F]) {
+      for (auto &Call : *CGNode) {
+        Function *called = Call.second->getFunction();
+        if (called->isDeclaration())
+          continue;
+        maskForCalls |= Visit(called, remainingMask, visited, CG);
+        if (remainingMask == 0)
+          return 0; // Nothing left to search for.
+      }
+    }
+
+    // Mask of incompatibilities introduced by this function.
+    uint32_t conflictsIntroduced =
+        remainingMask & maskForThisFunction & ~maskForCalls;
+    if (conflictsIntroduced) {
+      // This function introduces at least one conflict.
+      DiagnoseConflicts(F, conflictsIntroduced);
+      // Mask off diagnosed incompatibilities.
+      remainingMask &= ~conflictsIntroduced;
+    }
+    return maskForThisFunction;
+  }
+
+  void FindIncompatibleCall(const DxilModule::ShaderCompatInfo &compatInfo) {
+    uint32_t conflictMask = IdentifyConflict(compatInfo);
+    if (conflictMask == 0)
+      return;
+
+    CallGraph &CG = ValCtx.GetCallGraph();
+    llvm::SmallPtrSet<Function *, 8> visited;
+    Visit(EntryFn, conflictMask, visited, CG);
+  }
+};
+
+static void ValidateEntryCompatibility(ValidationContext &ValCtx) {
+  // Make sure functions called from each entry are compatible with that entry.
+  DxilModule &DM = ValCtx.DxilMod;
+  for (Function &F : DM.GetModule()->functions()) {
+    if (DM.HasDxilEntryProps(&F)) {
+      const DxilModule::ShaderCompatInfo *compatInfo =
+          DM.GetCompatInfoForFunction(&F);
+      DXASSERT(compatInfo, "otherwise, compat info not computed in module");
+      if (!compatInfo)
+        continue;
+
+      CompatibilityChecker checker(ValCtx, &F);
+      checker.FindIncompatibleCall(*compatInfo);
+    }
+  }
+}
+
 static void CheckPatchConstantSemantic(ValidationContext &ValCtx,
                                        const DxilEntryProps &EntryProps,
                                        EntryStatus &Status, Function *F) {
@@ -5900,7 +6117,7 @@ CalculateCallDepth(CallGraphNode *node,
 
 static void ValidateCallGraph(ValidationContext &ValCtx) {
   // Build CallGraph.
-  CallGraph CG(*ValCtx.DxilMod.GetModule());
+  CallGraph &CG = ValCtx.GetCallGraph();
 
   std::unordered_map<CallGraphNode *, unsigned> depthMap;
   std::unordered_set<CallGraphNode *> callStack;
@@ -6161,6 +6378,8 @@ HRESULT ValidateDxilModule(llvm::Module *pModule, llvm::Module *pDebugModule) {
 
   ValidateShaderFlags(ValCtx);
 
+  ValidateEntryCompatibility(ValCtx);
+
   ValidateEntrySignatures(ValCtx);
 
   ValidateUninitializedOutput(ValCtx);

tools/clang/test/CodeGenDXIL/hlsl/objects/NodeObjects/node-object-noinline.hlsl

@@ -15,7 +15,7 @@ EmptyNodeOutput outputNode)
 [NodeDispatchGrid(1, 1, 1)]
 [NumThreads(1, 1, 1)]
 void node_EmptyNodeOutput(
-	[MaxOutputRecords(1)] EmptyNodeOutput loadStressChild
+	[MaxRecords(1)] EmptyNodeOutput loadStressChild
 )
 {
 	loadStressEmptyRecWorker(wrapper(loadStressChild));

tools/clang/test/CodeGenDXIL/hlsl/objects/NodeObjects/node-objects-metdata.hlsl

@@ -301,7 +301,7 @@ void node_RWGroupNodeInputRecords([MaxRecords(4)] RWGroupNodeInputRecords<RECORD
 [NodeLaunch("thread")]
 void node_RWThreadNodeInputRecord(RWThreadNodeInputRecord<RECORD> input)
 {
-   Barrier(wrapper(input), 3);
+   Barrier(wrapper(input), 0);
 }
 
 //  ThreadNodeInputRecord
@@ -317,7 +317,7 @@ void node_RWThreadNodeInputRecord(RWThreadNodeInputRecord<RECORD> input)
 [NodeLaunch("thread")]
 void node_ThreadNodeInputRecord(ThreadNodeInputRecord<RECORD> input)
 {
-   Barrier(wrapper(input), 3);
+   Barrier(wrapper(input), 0);
 }
 
 

tools/clang/test/HLSLFileCheck/d3dreflect/rdat_mintarget/sm68_samplecmpgradbias.hlsl

@@ -60,7 +60,7 @@ void samplecmpbias() {
 // RDAT:   MinShaderTarget: 0x50068
 
 [shader("compute")]
-[numthreads(1,1,1)]
+[numthreads(4,1,1)]
 void samplecmpbias_compute(uint tidx : SV_GroupIndex) {
   samplecmpbias();
 }

tools/clang/test/HLSLFileCheck/hlsl/objects/Texture/Sample_node.hlsl

@@ -38,9 +38,11 @@ float2 ddy;
 bool clamp;
 
 [Shader("node")]
-[NodeLaunch("thread")]
+[NodeLaunch("broadcasting")]
+[NodeDispatchGrid(1, 1, 1)]
+[NumThreads(4, 1, 1)]
 void BackwardRef(
-  RWThreadNodeInputRecord<rec0> InputyMcInputFace,
+  RWDispatchNodeInputRecord<rec0> InputyMcInputFace,
   [MaxRecords(5)] NodeOutput<rec1> Output1,
   [MaxRecordsSharedWith(Output1)] NodeOutput<rec1> Output2)
 {

tools/clang/test/HLSLFileCheck/hlsl/workgraph/case117_barrier_memoryarg.hlsl

@@ -7,7 +7,9 @@ static const int a = 7;
 static const int b = 3;
 
 [Shader("node")]
-[NodeLaunch("thread")]
+[NodeLaunch("broadcasting")]
+[NodeDispatchGrid(1, 1, 1)]
+[NumThreads(1, 1, 1)]
 void node117_barrier_memoryarg()
 {
   // literal integer flag values
@@ -67,9 +69,9 @@ void node117_barrier_memoryarg()
 // Arg #1: ShaderKind Tag (8)
 // Arg #2: Node (15)
 // Arg #3: NodeLaunch Tag (13)
-// Arg #4: thread (3)
+// Arg #4: broadcasting (1)
 // ...
 // ------------------------------------------------------------------
 // CHECK: [[ATTRS]] = !{
-// CHECK-SAME: i32 8, i32 15, i32 13, i32 3
+// CHECK-SAME: i32 8, i32 15, i32 13, i32 1
 // CHECK-SAME: }

tools/clang/test/HLSLFileCheck/hlsl/workgraph/case118_barrier_objectarg.hlsl

@@ -37,12 +37,12 @@ void node02([MaxRecords(8)] GroupNodeInputRecords<RECORD> input)
 [NodeLaunch("thread")]
 void node03(RWThreadNodeInputRecord<RECORD> input)
 {
-   Barrier(input, 3);
+   Barrier(input, 0);
 }
 // CHECK: define void @node03() {
 // CHECK:   [[NODE03_A:%[0-9]+]] = call %dx.types.NodeRecordHandle @dx.op.createNodeInputRecordHandle(i32 {{[0-9]+}}, i32 0)  ; CreateNodeInputRecordHandle(MetadataIdx)
 // CHECK: [[ANN_NODE03_A:%[0-9]+]] =  call %dx.types.NodeRecordHandle @dx.op.annotateNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[NODE03_A]], %dx.types.NodeRecordInfo { i32 37, i32 4 })
-// CHECK:   call void @dx.op.barrierByNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[ANN_NODE03_A]], i32 3)  ; BarrierByNodeRecordHandle(object,SemanticFlags)
+// CHECK:   call void @dx.op.barrierByNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[ANN_NODE03_A]], i32 0)  ; BarrierByNodeRecordHandle(object,SemanticFlags)
 
 [Shader("node")]
 [NodeLaunch("coalescing")]
@@ -74,22 +74,22 @@ void node05([MaxRecords(5)] NodeOutput<RECORD> outputs)
 
 [Shader("node")]
 [NodeLaunch("thread")]
-void node06([MaxOutputRecords(5)] NodeOutput<RECORD> outputs)
+void node06([MaxRecords(5)] NodeOutput<RECORD> outputs)
 {
    ThreadNodeOutputRecords<RECORD> outrec = outputs.GetThreadNodeOutputRecords(3);
-   Barrier(outrec, 3);
+   Barrier(outrec, 0);
 }
 // CHECK: define void @node06() {
 // CHECK:   [[NODE06_A:%[0-9]+]] = call %dx.types.NodeHandle @dx.op.createNodeOutputHandle(i32 {{[0-9]+}}, i32 0)  ; CreateNodeOutputHandle(MetadataIdx)
 // CHECK:   [[ANN_NODE06_A:%[0-9]+]] = call %dx.types.NodeHandle @dx.op.annotateNodeHandle(i32 {{[0-9]+}}, %dx.types.NodeHandle [[NODE06_A]], %dx.types.NodeInfo { i32 6, i32 4 })
 // CHECK:   [[NODE06_B:%[0-9]+]] = call %dx.types.NodeRecordHandle @dx.op.allocateNodeOutputRecords(i32 {{[0-9]+}}, %dx.types.NodeHandle [[ANN_NODE06_A]], i32 3, i1 true)  ; AllocateNodeOutputRecords(output,numRecords,perThread)
 // CHECK:   [[ANN_NODE06_B:%[0-9]+]] = call %dx.types.NodeRecordHandle @dx.op.annotateNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[NODE06_B]], %dx.types.NodeRecordInfo { i32 38, i32 4 })
-// CHECK:   call void @dx.op.barrierByNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[ANN_NODE06_B]], i32 3)  ; BarrierByNodeRecordHandle(object,SemanticFlags)
+// CHECK:   call void @dx.op.barrierByNodeRecordHandle(i32 {{[0-9]+}}, %dx.types.NodeRecordHandle [[ANN_NODE06_B]], i32 0)  ; BarrierByNodeRecordHandle(object,SemanticFlags)
 
 [Shader("node")]
 [NodeLaunch("coalescing")]
 [NumThreads(256,1,3)]
-void node07([MaxOutputRecords(5)] NodeOutput<RECORD> outputs)
+void node07([MaxRecords(5)] NodeOutput<RECORD> outputs)
 {
    GroupNodeOutputRecords<RECORD> outrec = outputs.GetGroupNodeOutputRecords(1);
    Barrier(outrec, 3);

tools/clang/test/HLSLFileCheck/hlsl/workgraph/member_atomics.hlsl

@@ -71,7 +71,7 @@ void node03(NodeOutput<RECORD> output3)
 [Shader("node")]
 [NumThreads(1024,1,1)]
 [NodeLaunch("coalescing")]
-void node04([MaxOutputRecords(5)] NodeOutput<RECORD> outputs4)
+void node04([MaxRecords(5)] NodeOutput<RECORD> outputs4)
 {
    ThreadNodeOutputRecords<RECORD> outrec = outputs4.GetThreadNodeOutputRecords(1);
   // CHECK: getelementptr %struct.RECORD, %struct.RECORD addrspace(6)*