[Sema] Move more diagnostics over (#5992)

This PR moves over some more diagnostics from CGHLSLMS.cpp. Some
diagnostics are moved into DiagnoseEntryAttrAllowedOnStage, to
specifically diagnose when an attribute appears on a shader stage that
it shouldn't appear on. Other diagnostics that are covered in Sema are
removed in CGHLSLMS.
Fixes #5990

---------

Co-authored-by: Chris B <cbieneman@microsoft.com>

tools/clang/include/clang/AST/HlslTypes.h

@@ -440,6 +440,7 @@ const clang::ExtVectorType *
 ConvertHLSLVecMatTypeToExtVectorType(const clang::ASTContext &,
                                      clang::QualType);
 bool IsHLSLVecMatType(clang::QualType);
+clang::RecordDecl *GetRecordDeclFromNodeObjectType(clang::QualType ObjectTy);
 bool IsHLSLVecType(clang::QualType type);
 bool IsHLSLMatType(clang::QualType type);
 clang::QualType GetElementTypeOrType(clang::QualType type);

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7823,6 +7823,8 @@ def warn_hlsl_numthreads_group_size : Warning<
    InGroup<IgnoredAttributes>;
 def err_hlsl_wg_nodetrackrwinputsharing_missing : Error<
    "Use of FinishedCrossGroupSharing() requires NodeTrackRWInputSharing attribute to be specified on the record struct type">;
+def err_hlsl_wg_nodetrackrwinputsharing_invalid : Error<
+   "NodeTrackRWInputSharing attribute cannot be applied to Input Records that are not RWDispatchNodeInputRecord">;
 def err_hlsl_wg_input_kind : Error<
    "'%0' may not be used with %1 nodes (only %select{DispatchNodeInputRecord or RWDispatchNodeInputRecord|"
    "GroupNodeInputRecords, RWGroupNodeInputRecords, or EmptyNodeInput|ThreadNodeInputRecord or RWThreadNodeInputRecord}2)">;

tools/clang/lib/AST/HlslTypes.cpp

@@ -867,6 +867,24 @@ bool GetHLSLSubobjectKind(clang::QualType type,
   return false;
 }
 
+clang::RecordDecl *GetRecordDeclFromNodeObjectType(clang::QualType ObjectTy) {
+  ObjectTy = ObjectTy.getCanonicalType();
+  DXASSERT(IsHLSLNodeType(ObjectTy), "Expected Node Object type");
+  if (const CXXRecordDecl *CXXRD = ObjectTy->getAsCXXRecordDecl()) {
+
+    if (const ClassTemplateSpecializationDecl *templateDecl =
+            dyn_cast<ClassTemplateSpecializationDecl>(CXXRD)) {
+
+      auto &TemplateArgs = templateDecl->getTemplateArgs();
+      clang::QualType RecType = TemplateArgs[0].getAsType();
+      if (const RecordType *RT = RecType->getAs<RecordType>())
+        return RT->getDecl();
+    }
+  }
+
+  return nullptr;
+}
+
 bool IsHLSLRayQueryType(clang::QualType type) {
   type = type.getCanonicalType();
   if (const RecordType *RT = dyn_cast<RecordType>(type)) {

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -1402,15 +1402,6 @@ static DxilResource::Kind KeywordToKind(StringRef keyword) {
   return DxilResource::Kind::Invalid;
 }
 
-static void ReportMissingNodeDiag(DiagnosticsEngine &Diags,
-                                  const InheritableAttr *a) {
-  SourceLocation loc = a->getLocation();
-  unsigned DiagID = Diags.getCustomDiagID(
-      DiagnosticsEngine::Error, "Attribute %0 only applies to node shaders "
-                                "(indicated with '[shader(\"node\")]')");
-  Diags.Report(loc, DiagID) << a->getSpelling();
-}
-
 void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
   // Add hlsl intrinsic attr
   unsigned intrinsicOpcode;
@@ -1522,20 +1513,16 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
   };
 
   // Some diagnostic assumptions for shader attribute:
-  // - multiple shader attributes may exist in HLSL
   // - duplicate attribute of same kind is ok
-  // - node attribute only combinable with compute
   // - all attributes parsed before set from insertion or target shader model
-
   auto DiagShaderStage = [&priorShaderAttrLoc,
                           &Diags](clang::SourceLocation diagLoc,
                                   llvm::StringRef shaderStage,
-                                  ShaderStageSource source, bool bConflict) {
+                                  ShaderStageSource source) {
     bool bFromProfile = source == ShaderStageSource::Profile;
     unsigned DiagID = Diags.getCustomDiagID(
-        DiagnosticsEngine::Error,
+        DiagnosticsEngine::Error, "Invalid shader %select{profile|attribute}0");
-        "%select{Invalid|Conflicting}0 shader %select{profile|attribute}1");
+    Diags.Report(diagLoc, DiagID) << bFromProfile;
-    Diags.Report(diagLoc, DiagID) << bConflict << bFromProfile;
     if (priorShaderAttrLoc.isValid()) {
       unsigned DiagID = Diags.getCustomDiagID(
           DiagnosticsEngine::Note, "See conflicting shader attribute");
@@ -1547,7 +1534,7 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
       [&](clang::SourceLocation diagLoc, DXIL::ShaderKind shaderKind,
           llvm::StringRef shaderStage, ShaderStageSource source) {
         if (!SetStageFlag(shaderKind)) {
-          DiagShaderStage(diagLoc, shaderStage, source, false);
+          DiagShaderStage(diagLoc, shaderStage, source);
         }
         if (isEntry && isRay) {
           unsigned DiagID = Diags.getCustomDiagID(
@@ -1555,18 +1542,6 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
               "Ray function cannot be used as a global entry point");
           Diags.Report(diagLoc, DiagID);
         }
-        if (isEntry && isNode && !SM->IsCS()) {
-          unsigned DiagID =
-              Diags.getCustomDiagID(DiagnosticsEngine::Error,
-                                    "Node function as global entry point must "
-                                    "be compiled to compute shader target");
-          Diags.Report(diagLoc, DiagID);
-        }
-        if (funcProps->shaderKind != DXIL::ShaderKind::Invalid &&
-            funcProps->shaderKind != shaderKind) {
-          // Different kinds and not the node case, so it's a conflict.
-          DiagShaderStage(diagLoc, shaderStage, source, true);
-        }
         // Update shaderKind, unless we would be overriding one with node, so
         // when node+compute, kind = compute.  Other conflicts are diagnosed
         // above.
@@ -1841,79 +1816,46 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
   if (isNode) {
     // Default launch type is defined to be Broadcasting.
     funcProps->Node.LaunchType = DXIL::NodeLaunchType::Broadcasting;
-  }
 
-  // Assign function properties for all "node" attributes.
+    // Assign function properties for all "node" attributes.
-  if (const auto *pAttr = FD->getAttr<HLSLNodeLaunchAttr>()) {
+    if (const auto *pAttr = FD->getAttr<HLSLNodeLaunchAttr>()) {
-    if (isNode)
       funcProps->Node.LaunchType =
           ShaderModel::NodeLaunchTypeFromName(pAttr->getLaunchType());
-    else
+    }
-      ReportMissingNodeDiag(Diags, pAttr);
-  }
 
-  if (const auto *pAttr = FD->getAttr<HLSLNodeIsProgramEntryAttr>()) {
+    if (const auto *pAttr = FD->getAttr<HLSLNodeIsProgramEntryAttr>()) {
-    if (isNode)
       funcProps->Node.IsProgramEntry = true;
-    else
+    }
-      ReportMissingNodeDiag(Diags, pAttr);
-  }
 
-  if (const auto *pAttr = FD->getAttr<HLSLNodeIdAttr>()) {
+    if (const auto *pAttr = FD->getAttr<HLSLNodeIdAttr>()) {
-    if (isNode) {
       funcProps->NodeShaderID.Name = pAttr->getName().str();
       funcProps->NodeShaderID.Index = pAttr->getArrayIndex();
     } else {
-      ReportMissingNodeDiag(Diags, pAttr);
-    }
-
-  } else {
-    if (isNode) {
       funcProps->NodeShaderID.Name = FD->getName().str();
       funcProps->NodeShaderID.Index = 0;
     }
-  }
+    if (const auto *pAttr =
-  if (const auto *pAttr =
+            FD->getAttr<HLSLNodeLocalRootArgumentsTableIndexAttr>()) {
-          FD->getAttr<HLSLNodeLocalRootArgumentsTableIndexAttr>()) {
-    if (isNode)
       funcProps->Node.LocalRootArgumentsTableIndex = pAttr->getIndex();
-    else
+    }
-      ReportMissingNodeDiag(Diags, pAttr);
+    if (const auto *pAttr = FD->getAttr<HLSLNodeShareInputOfAttr>()) {
-  }
-  if (const auto *pAttr = FD->getAttr<HLSLNodeShareInputOfAttr>()) {
-    if (isNode) {
       funcProps->NodeShaderSharedInput.Name = pAttr->getName().str();
       funcProps->NodeShaderSharedInput.Index = pAttr->getArrayIndex();
-    } else {
-      ReportMissingNodeDiag(Diags, pAttr);
     }
-  }
+    if (const auto *pAttr = FD->getAttr<HLSLNodeDispatchGridAttr>()) {
-  if (const auto *pAttr = FD->getAttr<HLSLNodeDispatchGridAttr>()) {
-    if (isNode) {
       funcProps->Node.DispatchGrid[0] = pAttr->getX();
       funcProps->Node.DispatchGrid[1] = pAttr->getY();
       funcProps->Node.DispatchGrid[2] = pAttr->getZ();
-    } else {
-      ReportMissingNodeDiag(Diags, pAttr);
     }
-  }
+    if (const auto *pAttr = FD->getAttr<HLSLNodeMaxDispatchGridAttr>()) {
-  if (const auto *pAttr = FD->getAttr<HLSLNodeMaxDispatchGridAttr>()) {
-    if (isNode) {
       funcProps->Node.MaxDispatchGrid[0] = pAttr->getX();
       funcProps->Node.MaxDispatchGrid[1] = pAttr->getY();
       funcProps->Node.MaxDispatchGrid[2] = pAttr->getZ();
-    } else {
-      ReportMissingNodeDiag(Diags, pAttr);
     }
-  }
+    if (const auto *pAttr = FD->getAttr<HLSLNodeMaxRecursionDepthAttr>()) {
-  if (const auto *pAttr = FD->getAttr<HLSLNodeMaxRecursionDepthAttr>()) {
-    if (isNode)
       funcProps->Node.MaxRecursionDepth = pAttr->getCount();
-    else
+    }
-      ReportMissingNodeDiag(Diags, pAttr);
+    if (!FD->getAttr<HLSLNumThreadsAttr>()) {
-  }
-  if (!FD->getAttr<HLSLNumThreadsAttr>()) {
-    if (isNode) {
       // NumThreads wasn't specified.
       // For a Thread launch node the default is (1,1,1,) which we set here.
       // Other node launch types require NumThreads and an error will have
@@ -1927,13 +1869,6 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
   const unsigned profileAttributes =
       isCS + isHS + isDS + isGS + isVS + isPS + isRay + isMS + isAS + isNode;
 
-  // TODO: check this in front-end and report error.
-  if (profileAttributes > 1)
-    Diags.Report(
-        FD->getLocation(),
-        Diags.getCustomDiagID(DiagnosticsEngine::Error,
-                              "Invalid shader stage attribute combination"));
-
   if (isEntry) {
     switch (funcProps->shaderKind) {
     case ShaderModel::Kind::Compute:

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -11458,7 +11458,6 @@ void DiagnoseEntryAttrAllowedOnStage(clang::Sema *self,
   if (entryPointDecl->hasAttrs()) {
     for (Attr *pAttr : entryPointDecl->getAttrs()) {
       switch (pAttr->getKind()) {
-
       case clang::attr::HLSLWaveSize: {
         switch (shaderKind) {
         case DXIL::ShaderKind::Compute:
@@ -11471,6 +11470,22 @@ void DiagnoseEntryAttrAllowedOnStage(clang::Sema *self,
               << "compute or node";
           break;
         }
+        break;
+      }
+      case clang::attr::HLSLNodeLaunch:
+      case clang::attr::HLSLNodeIsProgramEntry:
+      case clang::attr::HLSLNodeId:
+      case clang::attr::HLSLNodeLocalRootArgumentsTableIndex:
+      case clang::attr::HLSLNodeShareInputOf:
+      case clang::attr::HLSLNodeDispatchGrid:
+      case clang::attr::HLSLNodeMaxDispatchGrid:
+      case clang::attr::HLSLNodeMaxRecursionDepth: {
+        if (shaderKind != DXIL::ShaderKind::Node) {
+          self->Diag(pAttr->getRange().getBegin(),
+                     diag::err_hlsl_attribute_unsupported_stage)
+              << pAttr->getSpelling() << "node";
+        }
+        break;
       }
       }
     }
@@ -15707,7 +15722,6 @@ void DiagnoseNodeEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName,
     auto *NodeArraySizeAttr = Param->getAttr<HLSLNodeArraySizeAttr>();
     auto *UnboundedSparseNodesAttr =
         Param->getAttr<HLSLUnboundedSparseNodesAttr>();
-
     // Check any node input is compatible with the node launch type
     if (hlsl::IsHLSLNodeInputType(ParamTy)) {
       InputCount++;
@@ -15792,19 +15806,16 @@ void DiagnoseNodeEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName,
               << HLSLNodeObjectAttr::ConvertRecordTypeToStr(Kind);
       }
     }
-
     HLSLMaxRecordsSharedWithAttr *ExistingMRSWA =
         Param->getAttr<HLSLMaxRecordsSharedWithAttr>();
     if (ExistingMRSWA) {
       StringRef sharedName = ExistingMRSWA->getName()->getName();
-      unsigned int ArgIdx = 0;
       bool Found = false;
-      while (ArgIdx < FD->getNumParams()) {
+      for (const ParmVarDecl *ParamDecl : FD->params()) {
-        const ParmVarDecl *ParamDecl = FD->getParamDecl(ArgIdx);
         // validation that MRSW doesn't reference its own parameter is
         // already done at
         // SemaHLSL.cpp:ValidateMaxRecordsSharedWithAttributes so we don't
-        // need to check that ArgIdx != Idx.
+        // need to check that we are on the same argument.
         if (ParamDecl->getName() == sharedName) {
           // now we need to check that this parameter has an output record type.
           hlsl::NodeFlags nodeFlags;
@@ -15816,7 +15827,6 @@ void DiagnoseNodeEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName,
             }
           }
         }
-        ArgIdx++;
       }
 
       if (!Found) {
@@ -15824,6 +15834,29 @@ void DiagnoseNodeEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName,
                diag::err_hlsl_maxrecordssharedwith_references_invalid_arg);
       }
     }
+
+    // Make sure NodeTrackRWInputSharing attribute cannot be applied to
+    // Input Records that are not RWDispatchNodeInputRecord
+    if (hlsl::IsHLSLNodeInputType(ParamTy)) {
+      hlsl::NodeFlags nodeFlags;
+      if (GetHLSLNodeIORecordType(Param, nodeFlags)) {
+        hlsl::NodeIOProperties node(nodeFlags);
+
+        // determine if the NodeTrackRWInputSharing is an attribute on the
+        // template type
+        clang::RecordDecl *RD = hlsl::GetRecordDeclFromNodeObjectType(ParamTy);
+        if (RD) {
+          // Emit a diagnostic if the record is not RWDispatchNode and
+          // if it has the NodeTrackRWInputSharing attribute
+          if (RD->hasAttr<HLSLNodeTrackRWInputSharingAttr>() &&
+              node.Flags.GetNodeIOKind() !=
+                  DXIL::NodeIOKind::RWDispatchNodeInputRecord) {
+            S.Diags.Report(Param->getLocation(),
+                           diag::err_hlsl_wg_nodetrackrwinputsharing_invalid);
+          }
+        }
+      }
+    }
   }
   return;
 }
@@ -15854,13 +15887,15 @@ void TryAddShaderAttrFromTargetProfile(Sema &S, FunctionDecl *FD,
     return;
   }
 
+  isActiveEntry = true;
+
   std::string profile = S.getLangOpts().HLSLProfile;
   const ShaderModel *SM = hlsl::ShaderModel::GetByName(profile.c_str());
   const llvm::StringRef fullName = ShaderModel::FullNameFromKind(SM->GetKind());
 
   // don't add the attribute for an invalid profile, like library
   if (fullName.empty()) {
-    return;
+    llvm_unreachable("invalid shader kind");
   }
 
   // At this point, we've found the active entry, so we'll take a note of that
@@ -15868,12 +15903,12 @@ void TryAddShaderAttrFromTargetProfile(Sema &S, FunctionDecl *FD,
   isActiveEntry = true;
 
   HLSLShaderAttr *currentShaderAttr = FD->getAttr<HLSLShaderAttr>();
-  // Don't add the attribute if it already exists as an attribute on the decl.
+  // Don't add the attribute if it already exists as an attribute on the decl,
-  // In the special case that the target profile is compute and the
+  // and emit an error.
-  // entry decl already has a node shader attr, don't do anything
   if (currentShaderAttr) {
     llvm::StringRef currentFullName = currentShaderAttr->getStage();
     if (currentFullName != fullName) {
+
       S.Diag(currentShaderAttr->getLocation(),
              diag::err_hlsl_profile_conflicts_with_shader_attribute)
           << fullName << profile << currentFullName << EntryPointName;

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

@@ -1,23 +0,0 @@
-// RUN: %dxc -T lib_6_8 %s | FileCheck %s
-// check that an error gets generated each time a "node" attribute gets used without the presence of 
-// the '[shader("node")]' attribute
-
-[Shader("compute")]
-[NodeIsProgramEntry]
-[NodeID("nodeName")]
-[NodeLocalRootArgumentsTableIndex(3)]
-[NodeShareInputOf("nodeName")]
-[NodeMaxRecursionDepth(31)]
-[NodeDispatchGrid(1, 1, 1)]
-[NodeMaxDispatchGrid(2,2,2)]
-[NumThreads(1, 1, 1)]
-void secondNode()
-{
-    // CHECK-DAG: Attribute nodeisprogramentry only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodeid only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodelocalrootargumentstableindex only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodeshareinputof only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodemaxrecursiondepth only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodedispatchgrid only applies to node shaders (indicated with '[shader("node")]')
-    // CHECK-DAG: Attribute nodemaxdispatchgrid only applies to node shaders (indicated with '[shader("node")]')
-}

tools/clang/test/SemaHLSL/hlsl/workgraph/entry_node_attr_on_non_compute_stage.hlsl

@@ -0,0 +1,20 @@
+// RUN: %dxc -T ps_6_6 -E main -verify  %s
+
+struct loadStressRecord {
+    uint x : SV_DispatchGrid;
+};
+
+void loadStressWorker(NodeOutput<loadStressRecord> x) {
+    return;
+}
+
+[Shader("node")] // expected-error{{entry type 'pixel' from profile 'ps_6_6' conflicts with shader attribute type 'node' on entry function 'main'.}}
+[NodeMaxDispatchGrid(3, 1, 1)]
+[NumThreads(16, 1, 1)]
+void main(    
+    DispatchNodeInputRecord<loadStressRecord> input,
+    [MaxRecords(16)] NodeOutput<loadStressRecord> loadStressChild
+)
+{
+    loadStressWorker(loadStressChild);
+}

tools/clang/test/SemaHLSL/hlsl/workgraph/missing_node_attribute_error_msg.hlsl

@@ -0,0 +1,16 @@
+// RUN: %dxc -T cs_6_6 -verify %s 
+// check that an error gets generated each time a "node" attribute gets used without the presence of 
+// the '[shader("node")]' attribute
+
+[Shader("compute")]
+[NodeIsProgramEntry] // expected-error{{attribute nodeisprogramentry only allowed on node shaders}}
+[NodeID("nodeName")] // expected-error{{attribute nodeid only allowed on node shaders}}
+[NodeLocalRootArgumentsTableIndex(3)] // expected-error{{attribute nodelocalrootargumentstableindex only allowed on node shaders}}
+[NodeShareInputOf("nodeName")] // expected-error{{attribute nodeshareinputof only allowed on node shaders}}
+[NodeMaxRecursionDepth(31)] // expected-error{{attribute nodemaxrecursiondepth only allowed on node shaders}}
+[NodeDispatchGrid(1, 1, 1)] // expected-error{{attribute nodedispatchgrid only allowed on node shaders}}
+[NodeMaxDispatchGrid(2,2,2)] // expected-error{{attribute nodemaxdispatchgrid only allowed on node shaders}}
+[NumThreads(1, 1, 1)]
+void secondNode()
+{    
+}

tools/clang/test/SemaHLSL/hlsl/workgraph/node_track_rw_input_sharing.hlsl

@@ -0,0 +1,18 @@
+// RUN: %dxc -T lib_6_8 -HV 2021 %s -verify
+
+struct [NodeTrackRWInputSharing] TRACKED_RECORD
+{
+  uint a;
+};
+
+//==============================================================================
+// Check Get[GroupShared]NodeOutput[Array]() intrinsics don't match with invalid
+// parameter types.
+
+[Shader("node")]
+[NumThreads(8,1,1)]
+[NodeLaunch("broadcasting")]
+[NodeDispatchGrid(8,1,1)]
+// expected-error@+1{{NodeTrackRWInputSharing attribute cannot be applied to Input Records that are not RWDispatchNodeInputRecord}}
+void node4_01(DispatchNodeInputRecord<TRACKED_RECORD> input) {
+}