Skip to content

Commit b315ab2

Browse files
bob80905github-actions[bot]tex3d
authored
Add errors for prohibited features in mesh nodes, add validation for allowed features (#6517)
Tests need to be added to mesh nodes preview to ensure that the features we would expect to disallow for broadcast node shaders are also disallowed for mesh nodes. Fixes #6479 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Tex Riddell <[email protected]>
1 parent 6986ad8 commit b315ab2

11 files changed

Lines changed: 313 additions & 15 deletions

File tree

docs/DXIL.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,6 +3040,7 @@ DECL.FNFLATTENPARAM Function parameters must not use struc
30403040
DECL.FNISCALLED Functions can only be used by call instructions
30413041
DECL.MULTIPLENODEINPUTS A node shader may not have more than one input record
30423042
DECL.NODELAUNCHINPUTTYPE Invalid input record type for node launch type
3043+
DECL.NODEOUTPUTONMESHLAUNCH Node outputs are not allowed on mesh launch node shaders.
30433044
DECL.NOTUSEDEXTERNAL External declaration should not be used
30443045
DECL.PARAMSTRUCT Callable function parameter must be struct type
30453046
DECL.PAYLOADSTRUCT Payload parameter must be struct type

lib/HLSL/DxilValidation.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3556,6 +3556,26 @@ static void ValidateFunctionBody(Function *F, ValidationContext &ValCtx) {
35563556
ValidateAsIntrinsics(F, ValCtx, dispatchMesh);
35573557
}
35583558

3559+
// Only in the case of Mesh Nodes, we should ensure that there do not exist
3560+
// any output records in the function argument list.
3561+
static void ValidateMeshNodeOutputRecord(Function *F,
3562+
ValidationContext &ValCtx) {
3563+
// if there are no function props or LaunchType is Invalid, there is nothing
3564+
// to do here
3565+
if (!ValCtx.DxilMod.HasDxilFunctionProps(F))
3566+
return;
3567+
auto &props = ValCtx.DxilMod.GetDxilFunctionProps(F);
3568+
3569+
if (props.OutputNodes.size() == 0)
3570+
return;
3571+
3572+
for (auto &output : props.OutputNodes) {
3573+
ValCtx.EmitFnFormatError(F, ValidationRule::DeclNodeOutputOnMeshLaunch,
3574+
{F->getName(), output.OutputID.Name});
3575+
}
3576+
return;
3577+
}
3578+
35593579
static void ValidateNodeInputRecord(Function *F, ValidationContext &ValCtx) {
35603580
// if there are no function props or LaunchType is Invalid, there is nothing
35613581
// to do here
@@ -3707,6 +3727,8 @@ static void ValidateFunction(Function &F, ValidationContext &ValCtx) {
37073727
if (ValCtx.DxilMod.HasDxilFunctionProps(&F) &&
37083728
ValCtx.DxilMod.GetDxilFunctionProps(&F).IsNode()) {
37093729
ValidateNodeInputRecord(&F, ValCtx);
3730+
if (ValCtx.DxilMod.GetDxilFunctionProps(&F).IsMeshNode())
3731+
ValidateMeshNodeOutputRecord(&F, ValCtx);
37103732
}
37113733

37123734
ValidateFunctionBody(&F, ValCtx);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7855,7 +7855,7 @@ def err_hlsl_wg_nodetrackrwinputsharing_invalid : Error<
78557855
"NodeTrackRWInputSharing attribute cannot be applied to Input Records that are not RWDispatchNodeInputRecord">;
78567856
def err_hlsl_wg_input_kind : Error<
78577857
"'%0' may not be used with %1 nodes (only %select{DispatchNodeInputRecord or RWDispatchNodeInputRecord|"
7858-
"GroupNodeInputRecords, RWGroupNodeInputRecords, or EmptyNodeInput|ThreadNodeInputRecord or RWThreadNodeInputRecord|DispatchNodeInputRecord}2)">;
7858+
"GroupNodeInputRecords, RWGroupNodeInputRecords, or EmptyNodeInput|ThreadNodeInputRecord or RWThreadNodeInputRecord|[RW]DispatchNodeInputRecord}2)">;
78597859
def err_hlsl_wg_attr_only_on_output : Error<
78607860
"attribute %0 may only be used with output nodes">;
78617861
def err_hlsl_wg_attr_only_on_output_or_input_record : Error<
@@ -7866,6 +7866,8 @@ def err_hlsl_wg_attr_only_on_output_array : Error<
78667866
"attribute %0 may only be used with node output arrays (NodeOutputArray or EmptyNodeOutputArray)">;
78677867
def err_hlsl_wg_attr_only_on_node: Error<
78687868
"attribute %0 may only be used on a node ">;
7869+
def err_hlsl_wg_node_output_in_mesh_node: Error<
7870+
"parameter %0 is a Node Output type, which is not allowed for mesh node shaders.">;
78697871
def err_hlsl_zero_sized_record : Error<
78707872
"record used in %0 may not have zero size">;
78717873
def err_hlsl_rwnodeinputrecord_sv_dispatch : Error<

tools/clang/lib/Sema/SemaHLSL.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15201,12 +15201,10 @@ static bool nodeInputIsCompatible(DXIL::NodeIOKind IOType,
1520115201
DXIL::NodeLaunchType launchType) {
1520215202
switch (IOType) {
1520315203
case DXIL::NodeIOKind::DispatchNodeInputRecord:
15204+
case DXIL::NodeIOKind::RWDispatchNodeInputRecord:
1520415205
return launchType == DXIL::NodeLaunchType::Broadcasting ||
1520515206
launchType == DXIL::NodeLaunchType::Mesh;
1520615207

15207-
case DXIL::NodeIOKind::RWDispatchNodeInputRecord:
15208-
return launchType == DXIL::NodeLaunchType::Broadcasting;
15209-
1521015208
case DXIL::NodeIOKind::GroupNodeInputRecords:
1521115209
case DXIL::NodeIOKind::RWGroupNodeInputRecords:
1521215210
case DXIL::NodeIOKind::EmptyInput:
@@ -15619,12 +15617,20 @@ void DiagnoseNodeEntry(Sema &S, FunctionDecl *FD, llvm::StringRef StageName,
1561915617
}
1562015618
}
1562115619

15622-
// Dignose node output.
15620+
// Diagnose node output.
1562315621
for (ParmVarDecl *PD : FD->params()) {
1562415622
QualType ParamType = PD->getType().getCanonicalType();
1562515623

1562615624
// Find parameter that is the node input record
1562715625
if (hlsl::IsHLSLNodeOutputType(ParamType)) {
15626+
// First, node output types are not allowed on
15627+
// mesh node shaders
15628+
if (NodeLaunchTy == DXIL::NodeLaunchType::Mesh) {
15629+
S.Diags.Report(PD->getLocation(),
15630+
diag::err_hlsl_wg_node_output_in_mesh_node)
15631+
<< PD->getName();
15632+
}
15633+
1562815634
// Node records are template types
1562915635
if (RecordDecl *NodeStructDecl =
1563015636
hlsl::GetRecordDeclFromNodeObjectType(ParamType)) {

tools/clang/test/CodeGenDXIL/hlsl/objects/NodeObjects/mesh-node-invalid-input-parameters.hlsl

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,6 @@ struct RECORD {
88
uint3 gtid;
99
};
1010

11-
[Shader("node")]
12-
[numthreads(4,4,4)]
13-
[NodeDispatchGrid(4,4,4)]
14-
[NodeLaunch("mesh")] // expected-note {{Launch type defined here}}
15-
[OutputTopology("line")]
16-
void node01_rw(RWDispatchNodeInputRecord<RECORD> input, // expected-error {{'RWDispatchNodeInputRecord' may not be used with mesh nodes (only DispatchNodeInputRecord)}}
17-
uint3 GTID : SV_GroupThreadID ) {
18-
input.Get().gtid = GTID;
19-
}
20-
2111
[Shader("node")]
2212
[numthreads(4,4,4)]
2313
[NodeMaxDispatchGrid(4,4,4)]
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
; RUN: not %dxv %s | FileCheck %s
2+
3+
; This test was taken from tools\clang\test\CodeGenDXIL\hlsl\shaders\node\mesh-node-no-invalid-input-record.hlsl
4+
; and compiled with Tlib_6_9, after disabling the diagnostic that was emitted
5+
; for incompatible input types.
6+
; The test is to ensure that the validator catches incorrect input record types in metadata,
7+
; as well as any node outputs.
8+
9+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
10+
target triple = "dxil-ms-dx"
11+
12+
define void @myFancyNode1() {
13+
ret void
14+
}
15+
16+
define void @myFancyNode2() {
17+
ret void
18+
}
19+
20+
define void @myFancyNode3() {
21+
ret void
22+
}
23+
24+
define void @myFancyNode4() {
25+
ret void
26+
}
27+
28+
!llvm.ident = !{!0}
29+
!dx.version = !{!1}
30+
!dx.valver = !{!1}
31+
!dx.shaderModel = !{!2}
32+
!dx.typeAnnotations = !{!3}
33+
!dx.entryPoints = !{!7, !8, !17, !22, !27}
34+
35+
!0 = !{!"dxc(private) 1.8.0.4467 (mesh-nodes-out-params, 278d6cb3bac9-dirty)"}
36+
!1 = !{i32 1, i32 9}
37+
!2 = !{!"lib", i32 6, i32 9}
38+
; CHECK: mesh node shader 'myFancyNode1' has incompatible input record type (should be DispatchNodeInputRecord)
39+
!3 = !{i32 1, void ()* @myFancyNode1, !4, void ()* @myFancyNode2, !4, void ()* @myFancyNode3, !4, void ()* @myFancyNode4, !4}
40+
!4 = !{!5}
41+
!5 = !{i32 0, !6, !6}
42+
!6 = !{}
43+
!7 = !{null, !"", null, null, null}
44+
!8 = !{void ()* @myFancyNode1, !"myFancyNode1", null, null, !9}
45+
!9 = !{i32 8, i32 15, i32 13, i32 4, i32 15, !10, i32 16, i32 -1, i32 18, !11, i32 65536, i32 1, i32 20, !12, i32 4, !15, i32 5, !16}
46+
!10 = !{!"myFancyNode1", i32 0}
47+
!11 = !{i32 2, i32 2, i32 2}
48+
!12 = !{!13}
49+
!13 = !{i32 1, i32 33, i32 2, !14}
50+
!14 = !{i32 0, i32 16, i32 2, i32 4}
51+
!15 = !{i32 4, i32 5, i32 6}
52+
!16 = !{i32 0}
53+
; CHECK: mesh node shader 'myFancyNode2' has incompatible input record type (should be DispatchNodeInputRecord)
54+
!17 = !{void ()* @myFancyNode2, !"myFancyNode2", null, null, !18}
55+
!18 = !{i32 8, i32 15, i32 13, i32 4, i32 15, !19, i32 16, i32 -1, i32 18, !11, i32 65536, i32 1, i32 20, !20, i32 4, !15, i32 5, !16}
56+
!19 = !{!"myFancyNode2", i32 0}
57+
!20 = !{!21}
58+
!21 = !{i32 1, i32 37, i32 2, !14}
59+
; CHECK: mesh node shader 'myFancyNode3' has incompatible input record type (should be DispatchNodeInputRecord)
60+
!22 = !{void ()* @myFancyNode3, !"myFancyNode3", null, null, !23}
61+
!23 = !{i32 8, i32 15, i32 13, i32 4, i32 15, !24, i32 16, i32 -1, i32 18, !11, i32 65536, i32 1, i32 20, !25, i32 4, !15, i32 5, !16}
62+
!24 = !{!"myFancyNode3", i32 0}
63+
!25 = !{!26}
64+
!26 = !{i32 1, i32 65, i32 2, !14}
65+
; CHECK: mesh node shader 'myFancyNode4' has incompatible input record type (should be DispatchNodeInputRecord)
66+
!27 = !{void ()* @myFancyNode4, !"myFancyNode4", null, null, !28}
67+
!28 = !{i32 8, i32 15, i32 13, i32 4, i32 15, !29, i32 16, i32 -1, i32 18, !11, i32 65536, i32 1, i32 20, !30, i32 4, !15, i32 5, !16}
68+
!29 = !{!"myFancyNode4", i32 0}
69+
!30 = !{!31}
70+
!31 = !{i32 1, i32 69, i32 2, !14}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// RUN: %dxc -Tlib_6_9 %s -verify
2+
3+
// Make sure invalid input records aren't allowed for mesh node shaders
4+
// or RWDispatchNodeInputRecord
5+
6+
struct MY_MATERIAL_RECORD
7+
{
8+
uint textureIndex;
9+
float3 normal;
10+
};
11+
12+
[Shader("node")]
13+
[OutputTopology("line")]
14+
// expected-note@+1{{Launch type defined here}}
15+
[NodeLaunch("mesh")]
16+
[NumThreads(4,5,6)]
17+
[NodeDispatchGrid(2,2,2)]
18+
void myFancyNode1(
19+
// expected-error@+1{{'ThreadNodeInputRecord' may not be used with mesh nodes (only [RW]DispatchNodeInputRecord}}
20+
ThreadNodeInputRecord<MY_MATERIAL_RECORD> myProgressCounter1
21+
)
22+
{
23+
}
24+
25+
[Shader("node")]
26+
[OutputTopology("line")]
27+
// expected-note@+1{{Launch type defined here}}
28+
[NodeLaunch("mesh")]
29+
[NumThreads(4,5,6)]
30+
[NodeDispatchGrid(2,2,2)]
31+
void myFancyNode2(
32+
// expected-error@+1{{'RWThreadNodeInputRecord' may not be used with mesh nodes (only [RW]DispatchNodeInputRecord)}}
33+
RWThreadNodeInputRecord<MY_MATERIAL_RECORD> myProgressCounter3
34+
)
35+
{
36+
}
37+
38+
39+
40+
[Shader("node")]
41+
[OutputTopology("line")]
42+
// expected-note@+1{{Launch type defined here}}
43+
[NodeLaunch("mesh")]
44+
[NumThreads(4,5,6)]
45+
[NodeDispatchGrid(2,2,2)]
46+
void myFancyNode3(
47+
// expected-error@+1{{'GroupNodeInputRecords' may not be used with mesh nodes (only [RW]DispatchNodeInputRecord)}}
48+
GroupNodeInputRecords<MY_MATERIAL_RECORD> myProgressCounter6
49+
)
50+
{
51+
}
52+
53+
54+
[Shader("node")]
55+
[OutputTopology("line")]
56+
// expected-note@+1{{Launch type defined here}}
57+
[NodeLaunch("mesh")]
58+
[NumThreads(4,5,6)]
59+
[NodeDispatchGrid(2,2,2)]
60+
void myFancyNode4(
61+
// expected-error@+1{{'RWGroupNodeInputRecords' may not be used with mesh nodes (only [RW]DispatchNodeInputRecord)}}
62+
RWGroupNodeInputRecords<MY_MATERIAL_RECORD> myProgressCounter8
63+
)
64+
{
65+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %dxc -Tlib_6_9 %s -verify
2+
3+
// Make sure output nodes aren't allowed in mesh node shaders
4+
5+
struct MY_INPUT_RECORD
6+
{
7+
float value;
8+
uint data;
9+
};
10+
11+
struct MY_RECORD
12+
{
13+
uint3 dispatchGrid : SV_DispatchGrid;
14+
// shader arguments:
15+
uint foo;
16+
float bar;
17+
};
18+
struct MY_MATERIAL_RECORD
19+
{
20+
uint textureIndex;
21+
float3 normal;
22+
};
23+
24+
[Shader("node")]
25+
[OutputTopology("line")]
26+
[NodeLaunch("mesh")]
27+
[NumThreads(4,5,6)]
28+
[NodeDispatchGrid(2,2,2)]
29+
void myFancyNode(
30+
31+
DispatchNodeInputRecord<MY_INPUT_RECORD> myInput,
32+
// expected-error@+1{{parameter myFascinatingNode is a Node Output type, which is not allowed for mesh node shaders.}}
33+
NodeOutput<MY_RECORD> myFascinatingNode,
34+
// expected-error@+1{{parameter myRecords is a Node Output type, which is not allowed for mesh node shaders.}}
35+
[NodeID("myNiftyNode",3)] [MaxRecords(4)] NodeOutput<MY_RECORD> myRecords,
36+
// expected-error@+1{{parameter myMaterials is a Node Output type, which is not allowed for mesh node shaders.}}
37+
[MaxRecordsSharedWith(myRecords)] [AllowSparseNodes] [NodeArraySize(63)] NodeOutputArray<MY_MATERIAL_RECORD> myMaterials,
38+
// an output that has empty record size
39+
// expected-error@+1{{parameter myProgressCounter is a Node Output type, which is not allowed for mesh node shaders.}}
40+
[MaxRecords(20)] EmptyNodeOutput myProgressCounter,
41+
// expected-error@+1{{parameter myProgressCounter2 is a Node Output type, which is not allowed for mesh node shaders.}}
42+
[MaxRecords(20)] EmptyNodeOutputArray myProgressCounter2
43+
)
44+
{
45+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
; RUN: not %dxv %s | FileCheck %s
2+
3+
; This test was taken from tools\clang\test\CodeGenDXIL\hlsl\shaders\node\mesh-node-no-outputs.hlsl
4+
; and compiled with Tlib_6_9, after changing the node launch type from mesh to broadcasting.
5+
; The launch type has been manually written to be mesh below.
6+
; The test is to ensure that the validator catches incorrect input record types in metadata,
7+
; as well as any node outputs.
8+
9+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
10+
target triple = "dxil-ms-dx"
11+
12+
define void @myFancyNode() {
13+
ret void
14+
}
15+
16+
!llvm.ident = !{!0}
17+
!dx.version = !{!1}
18+
!dx.valver = !{!1}
19+
!dx.shaderModel = !{!2}
20+
!dx.typeAnnotations = !{!3}
21+
!dx.entryPoints = !{!7, !8}
22+
23+
!0 = !{!"dxc(private) 1.8.0.4467 (mesh-nodes-out-params, 278d6cb3bac9-dirty)"}
24+
!1 = !{i32 1, i32 9}
25+
!2 = !{!"lib", i32 6, i32 9}
26+
!3 = !{i32 1, void ()* @myFancyNode, !4}
27+
!4 = !{!5}
28+
!5 = !{i32 0, !6, !6}
29+
!6 = !{}
30+
!7 = !{null, !"", null, null, null}
31+
; CHECK: error: node shader 'myFancyNode' has disallowed node output 'myFascinatingNode' for 'mesh' launch
32+
!8 = !{void ()* @myFancyNode, !"myFancyNode", null, null, !9}
33+
; change the 4th argument from 1->4, to set mesh node launch type
34+
!9 = !{i32 8, i32 15, i32 13, i32 4, i32 15, !10, i32 16, i32 -1, i32 18, !11, i32 20, !12, i32 21, !15, i32 4, !29, i32 5, !30}
35+
!10 = !{!"myFancyNode", i32 0}
36+
!11 = !{i32 2, i32 2, i32 2}
37+
!12 = !{!13}
38+
!13 = !{i32 1, i32 97, i32 2, !14}
39+
!14 = !{i32 0, i32 8, i32 2, i32 4}
40+
!15 = !{!16, !20, !22, !25, !27}
41+
!16 = !{i32 1, i32 6, i32 2, !17, i32 3, i32 0, i32 0, !19}
42+
!17 = !{i32 0, i32 20, i32 1, !18, i32 2, i32 4}
43+
!18 = !{i32 0, i32 5, i32 3}
44+
!19 = !{!"myFascinatingNode", i32 0}
45+
!20 = !{i32 1, i32 6, i32 2, !17, i32 3, i32 4, i32 0, !21}
46+
; CHECK: error: node shader 'myFancyNode' has disallowed node output 'myNiftyNode' for 'mesh' launch
47+
!21 = !{!"myNiftyNode", i32 3}
48+
!22 = !{i32 1, i32 22, i32 2, !23, i32 3, i32 0, i32 5, i32 63, i32 4, i32 1, i32 6, i1 true, i32 0, !24}
49+
!23 = !{i32 0, i32 16, i32 2, i32 4}
50+
; CHECK: error: node shader 'myFancyNode' has disallowed node output 'myMaterials' for 'mesh' launch
51+
!24 = !{!"myMaterials", i32 0}
52+
!25 = !{i32 1, i32 10, i32 3, i32 20, i32 0, !26}
53+
; CHECK: error: node shader 'myFancyNode' has disallowed node output 'myProgressCounter' for 'mesh' launch
54+
!26 = !{!"myProgressCounter", i32 0}
55+
!27 = !{i32 1, i32 26, i32 3, i32 20, i32 0, !28}
56+
; CHECK: error: node shader 'myFancyNode' has disallowed node output 'myProgressCounter2' for 'mesh' launch
57+
!28 = !{!"myProgressCounter2", i32 0}
58+
!29 = !{i32 4, i32 5, i32 6}
59+
!30 = !{i32 0}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: %dxc -Tlib_6_9 %s -verify
2+
3+
// Make sure RWDispatchNodeInputRecord input records are allowed for mesh node shaders
4+
5+
// expected-no-diagnostics
6+
7+
struct MY_MATERIAL_RECORD
8+
{
9+
uint textureIndex;
10+
float3 normal;
11+
};
12+
13+
[Shader("node")]
14+
[OutputTopology("line")]
15+
[NodeLaunch("mesh")]
16+
[NumThreads(4,5,6)]
17+
[NodeDispatchGrid(2,2,2)]
18+
void myFancyNode1(
19+
RWDispatchNodeInputRecord<MY_MATERIAL_RECORD> myProgressCounter1
20+
)
21+
{
22+
}
23+
24+
[Shader("node")]
25+
[OutputTopology("line")]
26+
[NodeLaunch("mesh")]
27+
[NumThreads(4,5,6)]
28+
[NodeDispatchGrid(2,2,2)]
29+
void myFancyNode2(
30+
globallycoherent RWDispatchNodeInputRecord<MY_MATERIAL_RECORD> myProgressCounter1
31+
)
32+
{
33+
}

0 commit comments

Comments
 (0)