Move MaxRecords vs MaxRecordsSharedWith diagnostic

A node output parameter may not have both the MaxRecords and MaxRecordsShared with attribute. Moved the error diagnostic for this case from CodeGen to Sema, and replaced the FileCheck test with a verifier test case.
microsoft · Jul 27, 2023 · f3f7419 · f3f7419
1 parent 680ab66
commit f3f7419
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 43 deletions.
diff --git a/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td b/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7818,6 +7818,8 @@ def err_hlsl_maxrecursiondepth_exceeded : Error<
    "NodeMaxRecursionDepth may not exceed 32">;
 def err_hlsl_too_many_node_inputs : Error<
    "Node shader '%0' may not have more than one input record">;
+def err_hlsl_incompatible_param_attr : Error<
+   "parameter '%0' may not have both '%1' and '%2'">;
 // HLSL Change Ends
 
 // SPIRV Change Starts

diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp
@@ -2523,14 +2523,8 @@ void CGMSHLSLRuntime::AddHLSLFunctionInfo(Function *F, const FunctionDecl *FD) {
       }
       node.MaxRecordsSharedWith = ix;
     }
-    if (const auto *Attr = parmDecl->getAttr<HLSLMaxRecordsAttr>()) {
-      if (node.MaxRecordsSharedWith >= 0) {
-        Diags.Report(parmDecl->getLocation(), Diags.getCustomDiagID(
-          DiagnosticsEngine::Error,
-          "Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter."));
-      }
+    if (const auto *Attr = parmDecl->getAttr<HLSLMaxRecordsAttr>())
       node.MaxRecords = Attr->getMaxCount();
-    }
   }
 
   if (inputPatchCount > 1) {

diff --git a/tools/clang/lib/Sema/SemaHLSL.cpp b/tools/clang/lib/Sema/SemaHLSL.cpp
@@ -11462,6 +11462,12 @@ class WorkGraphVisitor : public RecursiveASTVisitor<WorkGraphVisitor> {
           << funcName << P->getSourceRange();
       }
     }
+    // MaxRecords and MaxRecordsSharedWith are mutually exclusive
+    if (auto *maxShared = P->getAttr<HLSLMaxRecordsSharedWithAttr>())
+      if (auto *maxRecs = P->getAttr<HLSLMaxRecordsAttr>())
+        S.Diags.Report(P->getLocation(), diag::err_hlsl_incompatible_param_attr)
+          << P->getName() << maxShared->getSpelling() << maxRecs->getSpelling()
+          << P->getSourceRange();
 
     return true;
   }

diff --git a/tools/clang/test/HLSL/workgraph/node_input_compatibility.hlsl b/tools/clang/test/HLSL/workgraph/node_input_compatibility.hlsl
@@ -113,3 +113,9 @@ void node17(ThreadNodeInputRecord<RECORD> input,
             NodeOutput<RECORD> output,
             RWThreadNodeInputRecord<RECORD> input2)  // expected-error {{Node shader 'node17' may not have more than one input record}}
 { }
+
+[Shader("node")]
+[NodeLaunch("thread")]
+void node18(NodeOutput<RECORD> firstOut,
+            [MaxRecords(5)][MaxRecordsSharedWith(firstOut)] NodeOutput<RECORD> secondOut) // expected-error {{parameter 'secondOut' may not have both 'maxrecordssharedwith' and 'MaxRecords'}}
+{ }
diff --git a/...est/HLSLFileCheck/hlsl/workgraph/case092_maxoutputrecords_maxoutputrecordssharedwith.hlsl b/...est/HLSLFileCheck/hlsl/workgraph/case092_maxoutputrecords_maxoutputrecordssharedwith.hlsl
diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/nodes/max_output_records_duplicate1.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/nodes/max_output_records_duplicate1.hlsl
@@ -3,10 +3,10 @@
 // Duplicate MaxRecords info with matching limits
 
 
-// CHECK: 28:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 29:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 30:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 31:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
+// CHECK: 28:68: error: parameter 'Output2' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 29:68: error: parameter 'Output3' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 30:68: error: parameter 'Output4' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 31:68: error: parameter 'Output5' may not have both 'maxrecordssharedwith' and 'MaxRecords'
 
 struct rec0
 {

diff --git a/tools/clang/test/HLSLFileCheck/shader_targets/nodes/max_output_records_duplicate2.hlsl b/tools/clang/test/HLSLFileCheck/shader_targets/nodes/max_output_records_duplicate2.hlsl
@@ -2,10 +2,10 @@
 
 // Duplicate MaxRecords info with mismatching limits
 
-// CHECK: 27:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 28:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 29:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
-// CHECK: 30:68: error: Only one of MaxRecords or MaxRecordsSharedWith may be specified to the same parameter.
+// CHECK: 27:68: error: parameter 'Output2' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 28:68: error: parameter 'Output3' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 29:68: error: parameter 'Output4' may not have both 'maxrecordssharedwith' and 'MaxRecords'
+// CHECK: 30:68: error: parameter 'Output5' may not have both 'maxrecordssharedwith' and 'MaxRecords'
 
 struct rec0
 {