Skip to content

Commit

Permalink
Require valid outputcontrolpoints on Hull shader
Browse files Browse the repository at this point in the history
According to the documentation: "A hull shader is implemented with an
HLSL function, and has the following properties: [...] The shader output
is between 1 and 32 control points".

https://learn.microsoft.com/en-us/windows/win32/direct3d11/direct3d-11-advanced-stages-tessellation#hull-shader-stage

This change adds a check to verify that the outputcontrolpoints
attribute is set on Hull shaders and has an argument in the valid range.

Fixes #3733 (by making the error consistent between backends)
  • Loading branch information
sudonatalie committed Jan 24, 2024
1 parent dcb618a commit 94559ae
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 8 deletions.
2 changes: 2 additions & 0 deletions tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -7895,6 +7895,8 @@ def err_hlsl_inputpatch_size: Error<
def err_hlsl_outputpatch_size: Error<
"OutputPatch element count must be greater than 0">;
def note_hlsl_node_array : Note<"'%0' cannot be used as an array; did you mean '%0Array'?">;
def err_hlsl_controlpoints_size: Error<
"number of control points %0 is outside the valid range of [1..32]">;
// HLSL Change Ends

// SPIRV Change Starts
Expand Down
20 changes: 16 additions & 4 deletions tools/clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13098,11 +13098,20 @@ void hlsl::HandleDeclAttributeForHLSL(Sema &S, Decl *D, const AttributeList &A,
ValidateAttributeStringArg(S, A, /*validate strings*/ nullptr),
A.getAttributeSpellingListIndex());
break;
case AttributeList::AT_HLSLOutputControlPoints:
declAttr = ::new (S.Context) HLSLOutputControlPointsAttr(
A.getRange(), S.Context, ValidateAttributeIntArg(S, A),
A.getAttributeSpellingListIndex());
case AttributeList::AT_HLSLOutputControlPoints: {
// Hull shader output must be between 1 and 32 control points.
int outputControlPoints = ValidateAttributeIntArg(S, A);
if (outputControlPoints >= 1 && outputControlPoints <= 32) {
declAttr = ::new (S.Context) HLSLOutputControlPointsAttr(
A.getRange(), S.Context, outputControlPoints,
A.getAttributeSpellingListIndex());
} else {
S.Diags.Report(A.getLoc(), diag::err_hlsl_controlpoints_size)
<< outputControlPoints << A.getRange();
return;
}
break;
}
case AttributeList::AT_HLSLOutputTopology:
declAttr = ::new (S.Context) HLSLOutputTopologyAttr(
A.getRange(), S.Context,
Expand Down Expand Up @@ -15207,6 +15216,9 @@ void DiagnoseHullEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName) {
if (!(FD->getAttr<HLSLOutputTopologyAttr>()))
S.Diags.Report(FD->getLocation(), diag::err_hlsl_missing_attr)
<< StageName << "outputtopology";
if (!(FD->getAttr<HLSLOutputControlPointsAttr>()))
S.Diags.Report(FD->getLocation(), diag::err_hlsl_missing_attr)
<< StageName << "outputcontrolpoints";

for (const auto *param : FD->params()) {
if (!hlsl::IsHLSLInputPatchType(param->getType()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ float4 fooey()
// CHECK: error: patch constant function 'NotFooey' must be defined
[patchconstantfunc("NotFooey")]
[outputtopology("point")]
[outputcontrolpoints(1)]
float4 main(float a : A, float b:B) : SV_TARGET
{
float4 f = b;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,6 @@ Texture2D<float4> tex1[10] : register( t20, space10 );
|-RegisterAssignment <col:30> register(t20, space10)
*/

[outputcontrolpoints(-1)] // expected-warning {{attribute 'outputcontrolpoints' must have a uint literal argument}} fxc-pass {{}}
void all_wrong() { }

[domain("quad")]
/*verify-ast
HLSLDomainAttr <col:2, col:15> "quad"
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/test/SemaHLSL/attributes.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ Texture2D<float4> tex1[10] : register( t20, space10 );
[domain(123)] // expected-error {{attribute 'domain' must have a string literal argument}} fxc-pass {{}}
[partitioning()] // expected-error {{'partitioning' attribute takes one argument}} fxc-error {{X3000: syntax error: unexpected token ')'}}
[outputtopology("not_triangle_cw")] // expected-error {{attribute 'outputtopology' must have one of these values: point,line,triangle,triangle_cw,triangle_ccw}} fxc-pass {{}}
[outputcontrolpoints(-1)] // expected-warning {{attribute 'outputcontrolpoints' must have a uint literal argument}} fxc-pass {{}}
[outputcontrolpoints(-1)] // expected-warning {{attribute 'outputcontrolpoints' must have a uint literal argument}} expected-error {{number of control points -1 is outside the valid range of [1..32]}} fxc-pass {{}}
[patchconstantfunc("PatchFoo", "ExtraArgument")] // expected-error {{'patchconstantfunc' attribute takes one argument}} fxc-pass {{}}
void all_wrong() { }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ HSPerPatchData HSPerPatchFunc1() /* expected-error{{recursive functions are not
[shader("hull")]
[patchconstantfunc("HSPerPatchFunc1")]
[outputtopology("point")]
[outputcontrolpoints(1)]
float4 main(float a : A, float b:B) : SV_TARGET
{
float4 f = b;
Expand Down

0 comments on commit 94559ae

Please sign in to comment.