From 3aabc019ecf21a4893e5d347191845f285b09bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20Gau=C3=ABr?= Date: Fri, 29 Mar 2024 11:13:31 +0100 Subject: [PATCH] [Sema] Move outputcontrolpoints check to Sema (#6460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DXIL had a check in the backend for those values. Moving it to Sema to share it between DXIL and SPIR-V. Fixes #6387 Signed-off-by: Nathan Gauër --- .../clang/Basic/DiagnosticSemaKinds.td | 4 +- .../lib/CodeGen/CGHLSLMSFinishCodeGen.cpp | 19 -------- tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp | 48 ++++++++++++++++--- ...semantic.hull-output.size-mismatch.hs.hlsl | 22 +++++++++ ...semantic.hull-output.size-mismatch.hs.hlsl | 22 +++++++++ .../HLSLFileCheck/samples/SimpleHs12.hlsl | 3 +- 6 files changed, 88 insertions(+), 30 deletions(-) create mode 100644 tools/clang/test/CodeGenDXIL/semantic.hull-output.size-mismatch.hs.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/semantic.hull-output.size-mismatch.hs.hlsl diff --git a/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td b/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td index f7915c5d9..6e103f1fb 100644 --- a/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7837,8 +7837,8 @@ def err_hlsl_missing_patch_constant_function : Error< "patch constant function '%0' must be defined">; def err_hlsl_patch_reachability_not_allowed : Error< "%select{patch constant|entry}0 function '%1' should not be reachable from %select{patch constant|entry}2 function '%3'">; -def err_hlsl_patch_input_size_mismatch : Error< - "Patch constant function's input patch input should have %0 elements, but has %1.">; +def err_hlsl_patch_size_mismatch : Error< + "Patch constant function's %0 patch input should have %1 elements, but has %2.">; def warn_hlsl_structurize_exits_lifetime_markers_conflict : Warning < "structurize-returns skipped function '%0' due to incompatibility with lifetime markers. Use -disable-lifetime-markers to enable structurize-exits on this function.">, InGroup< HLSLStructurizeExitsLifetimeMarkersConflict >; diff --git a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp index c44f41ab7..012fba1ed 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp @@ -1475,7 +1475,6 @@ void SetPatchConstantFunctionWithAttr( HLM.HasDxilFunctionProps(EntryFunc.Func), " else AddHLSLFunctionInfo did not save the dxil function props for the " "HS entry."); - DxilFunctionProps *HSProps = &HLM.GetDxilFunctionProps(EntryFunc.Func); HLM.SetPatchConstantFunctionForHS(EntryFunc.Func, patchConstFunc); DXASSERT_NOMSG(patchConstantFunctionPropsMap.count(patchConstFunc)); // Check no inout parameter for patch constant function. @@ -1491,24 +1490,6 @@ void SetPatchConstantFunctionWithAttr( Diags.Report(Entry->second.SL, DiagID) << funcName; } } - - // Input/Output control point validation. - if (patchConstantFunctionPropsMap.count(patchConstFunc)) { - const DxilFunctionProps &patchProps = - *patchConstantFunctionPropsMap[patchConstFunc]; - if (patchProps.ShaderProps.HS.outputControlPoints != 0 && - patchProps.ShaderProps.HS.outputControlPoints != - HSProps->ShaderProps.HS.outputControlPoints) { - clang::DiagnosticsEngine &Diags = CGM.getDiags(); - unsigned DiagID = - Diags.getCustomDiagID(clang::DiagnosticsEngine::Error, - "Patch constant function's output patch input " - "should have %0 elements, but has %1."); - Diags.Report(Entry->second.SL, DiagID) - << HSProps->ShaderProps.HS.outputControlPoints - << patchProps.ShaderProps.HS.outputControlPoints; - } - } } void SetPatchConstantFunction( diff --git a/tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp b/tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp index 5937d5303..149c6b32e 100644 --- a/tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp +++ b/tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp @@ -344,6 +344,25 @@ getFunctionInputPatchCount(const FunctionDecl *function) { return std::nullopt; } +std::optional +getFunctionOutputPatchCount(const FunctionDecl *function) { + for (const auto *param : function->params()) { + if (!hlsl::IsHLSLOutputPatchType(param->getType())) + continue; + return hlsl::GetHLSLOutputPatchCount(param->getType()); + } + + return std::nullopt; +} + +std::optional +getFunctionOutputControlPointsCount(const FunctionDecl *function) { + if (const auto *Attr = function->getAttr()) { + return Attr->getCount(); + } + return std::nullopt; +} + } // namespace void hlsl::DiagnoseTranslationUnit(clang::Sema *self) { @@ -475,13 +494,28 @@ void hlsl::DiagnoseTranslationUnit(clang::Sema *self) { } } - auto hullPatchCount = getFunctionInputPatchCount(pPatchFnDecl); - auto functionPatchCount = getFunctionInputPatchCount(FDecl); - if (hullPatchCount.has_value() && functionPatchCount.has_value() && - hullPatchCount.value() != functionPatchCount.value()) { - self->Diag(pPatchFnDecl->getSourceRange().getBegin(), - diag::err_hlsl_patch_input_size_mismatch) - << functionPatchCount.value() << hullPatchCount.value(); + // Input/Output control point validation. + { + auto hullPatchCount = getFunctionInputPatchCount(pPatchFnDecl); + auto functionPatchCount = getFunctionInputPatchCount(FDecl); + if (hullPatchCount.has_value() && functionPatchCount.has_value() && + hullPatchCount.value() != functionPatchCount.value()) { + self->Diag(pPatchFnDecl->getSourceRange().getBegin(), + diag::err_hlsl_patch_size_mismatch) + << "input" << functionPatchCount.value() + << hullPatchCount.value(); + } + } + { + auto hullPatchCount = getFunctionOutputPatchCount(pPatchFnDecl); + auto functionPatchCount = getFunctionOutputControlPointsCount(FDecl); + if (hullPatchCount.has_value() && functionPatchCount.has_value() && + hullPatchCount.value() != functionPatchCount.value()) { + self->Diag(pPatchFnDecl->getSourceRange().getBegin(), + diag::err_hlsl_patch_size_mismatch) + << "output" << functionPatchCount.value() + << hullPatchCount.value(); + } } } diff --git a/tools/clang/test/CodeGenDXIL/semantic.hull-output.size-mismatch.hs.hlsl b/tools/clang/test/CodeGenDXIL/semantic.hull-output.size-mismatch.hs.hlsl new file mode 100644 index 000000000..7b1cb96dc --- /dev/null +++ b/tools/clang/test/CodeGenDXIL/semantic.hull-output.size-mismatch.hs.hlsl @@ -0,0 +1,22 @@ +// RUN: %dxc -T hs_6_0 -E main %s -verify + + +struct ControlPoint { float4 position : POSITION; }; + +struct HullPatchOut { + float edge [3] : SV_TessFactor; + float inside : SV_InsideTessFactor; +}; + +HullPatchOut HullConst (InputPatch v, OutputPatch outpoints) { /* expected-error{{Patch constant function's output patch input should have 5 elements, but has 2.}} */ + return (HullPatchOut)0; +} + +[domain("tri")] +[partitioning("fractional_odd")] +[outputtopology("triangle_cw")] +[patchconstantfunc("HullConst")] +[outputcontrolpoints(5)] +ControlPoint main(InputPatch v, uint id : SV_OutputControlPointID) { + return v[id]; +} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.hull-output.size-mismatch.hs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.hull-output.size-mismatch.hs.hlsl new file mode 100644 index 000000000..b8f311891 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/semantic.hull-output.size-mismatch.hs.hlsl @@ -0,0 +1,22 @@ +// RUN: %dxc -T hs_6_0 -E main %s -spirv -verify + + +struct ControlPoint { float4 position : POSITION; }; + +struct HullPatchOut { + float edge [3] : SV_TessFactor; + float inside : SV_InsideTessFactor; +}; + +HullPatchOut HullConst (InputPatch v, OutputPatch outpoints) { /* expected-error{{Patch constant function's output patch input should have 5 elements, but has 2.}} */ + return (HullPatchOut)0; +} + +[domain("tri")] +[partitioning("fractional_odd")] +[outputtopology("triangle_cw")] +[patchconstantfunc("HullConst")] +[outputcontrolpoints(5)] +ControlPoint main(InputPatch v, uint id : SV_OutputControlPointID) { + return v[id]; +} diff --git a/tools/clang/test/HLSLFileCheck/samples/SimpleHs12.hlsl b/tools/clang/test/HLSLFileCheck/samples/SimpleHs12.hlsl index e64003125..cf80241f8 100644 --- a/tools/clang/test/HLSLFileCheck/samples/SimpleHs12.hlsl +++ b/tools/clang/test/HLSLFileCheck/samples/SimpleHs12.hlsl @@ -1,7 +1,6 @@ // RUN: %dxc -E main -T hs_6_0 %s | FileCheck %s // CHECK-DAG: Patch Constant function HSPerPatchFunc should not have inout param -// CHECK-DAG: Patch constant function's output patch input should have 3 elements, but has 5. //-------------------------------------------------------------------------------------- // SimpleTessellation.hlsl @@ -62,7 +61,7 @@ HSPerPatchData HSPerPatchFunc( const InputPatch< PSSceneIn, 12 > points, OutputP [partitioning("fractional_odd")] [outputtopology("triangle_cw")] [patchconstantfunc("HSPerPatchFunc")] -[outputcontrolpoints(3)] +[outputcontrolpoints(5)] HSPerVertexData main(const uint id : SV_OutputControlPointID, const InputPatch< PSSceneIn, 12 > points ) {