Skip to content

Commit 420e2f5

Browse files
inbelicinbelic
authored
[DirectX] Add missing verifications during validate of DXILRootSignature (#147111)
This pr resolves some discrepancies in verification during `validate` in `DXILRootSignature.cpp`. Note: we don't add a backend test for version 1.0 flag values because it treats the struct as though there is no flags value. However, this will be used when we use the verifications in the frontend. - Updates `verifyDescriptorFlag` to check for valid flags based on version, as reflected [here](llvm/wg-hlsl#297) - Add test to demonstrate updated flag verifications - Adds `verifyNumDescriptors` to the validation of `DescriptorRange`s - Add a test to demonstrate `numDescriptors` verification - Updates a number of tests that mistakenly had an invalid `numDescriptors` specified Resolves: #147107
1 parent d5da826 commit 420e2f5

13 files changed

+82
-18
lines changed

‎llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ bool verifyRootFlag(uint32_t Flags);
2727
bool verifyVersion(uint32_t Version);
2828
bool verifyRegisterValue(uint32_t RegisterValue);
2929
bool verifyRegisterSpace(uint32_t RegisterSpace);
30-
bool verifyDescriptorFlag(uint32_t Flags);
30+
bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal);
3131
bool verifyRangeType(uint32_t Type);
3232
bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
3333
uint32_t FlagsVal);
34+
bool verifyNumDescriptors(uint32_t NumDescriptors);
3435
bool verifySamplerFilter(uint32_t Value);
3536
bool verifyAddress(uint32_t Address);
3637
bool verifyMipLODBias(float MipLODBias);

‎llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,24 @@ bool verifyRegisterSpace(uint32_t RegisterSpace) {
3232
return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF);
3333
}
3434

35-
bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; }
35+
bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal) {
36+
using FlagT = dxbc::RootDescriptorFlags;
37+
FlagT Flags = FlagT(FlagsVal);
38+
if (Version == 1)
39+
return Flags == FlagT::DataVolatile;
40+
41+
assert(Version == 2 && "Provided invalid root signature version");
42+
43+
// The data-specific flags are mutually exclusive.
44+
FlagT DataFlags = FlagT::DataVolatile | FlagT::DataStatic |
45+
FlagT::DataStaticWhileSetAtExecute;
46+
47+
if (popcount(llvm::to_underlying(Flags & DataFlags)) > 1)
48+
return false;
49+
50+
// Only a data flag or no flags is valid
51+
return (Flags | DataFlags) == DataFlags;
52+
}
3653

3754
bool verifyRangeType(uint32_t Type) {
3855
switch (Type) {
@@ -108,6 +125,10 @@ bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
108125
return (Flags & ~Mask) == FlagT::None;
109126
}
110127

128+
bool verifyNumDescriptors(uint32_t NumDescriptors) {
129+
return NumDescriptors > 0;
130+
}
131+
111132
bool verifySamplerFilter(uint32_t Value) {
112133
switch (Value) {
113134
#define FILTER(Num, Val) case llvm::to_underlying(dxbc::SamplerFilter::Val):

‎llvm/lib/Target/DirectX/DXILRootSignature.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,9 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
432432
return reportValueError(Ctx, "RegisterSpace", Descriptor.RegisterSpace);
433433

434434
if (RSD.Version > 1) {
435-
if (!llvm::hlsl::rootsig::verifyDescriptorFlag(Descriptor.Flags))
436-
return reportValueError(Ctx, "DescriptorRangeFlag", Descriptor.Flags);
435+
if (!llvm::hlsl::rootsig::verifyRootDescriptorFlag(RSD.Version,
436+
Descriptor.Flags))
437+
return reportValueError(Ctx, "RootDescriptorFlag", Descriptor.Flags);
437438
}
438439
break;
439440
}
@@ -447,6 +448,9 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
447448
if (!llvm::hlsl::rootsig::verifyRegisterSpace(Range.RegisterSpace))
448449
return reportValueError(Ctx, "RegisterSpace", Range.RegisterSpace);
449450

451+
if (!llvm::hlsl::rootsig::verifyNumDescriptors(Range.NumDescriptors))
452+
return reportValueError(Ctx, "NumDescriptors", Range.NumDescriptors);
453+
450454
if (!llvm::hlsl::rootsig::verifyDescriptorRangeFlag(
451455
RSD.Version, Range.RangeType, Range.Flags))
452456
return reportValueError(Ctx, "DescriptorFlag", Range.Flags);

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1313
!2 = !{ ptr @main, !3, i32 1 } ; function, root signature
1414
!3 = !{ !5 } ; list of root signature elements
1515
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
16-
!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 }
16+
!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
1717
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
1818

1919

@@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
3333
; DXC-NEXT: RangesOffset: 44
3434
; DXC-NEXT: Ranges:
3535
; DXC-NEXT: - RangeType: 3
36-
; DXC-NEXT: NumDescriptors: 0
36+
; DXC-NEXT: NumDescriptors: 1
3737
; DXC-NEXT: BaseShaderRegister: 1
3838
; DXC-NEXT: RegisterSpace: 0
3939
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1616
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
1717
!3 = !{ !5 } ; list of root signature elements
1818
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
19-
!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 22 }
19+
!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 22 }
2020
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-NumDescriptors.ll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
2+
3+
target triple = "dxil-unknown-shadermodel6.0-compute"
4+
5+
; CHECK: error: Invalid value for NumDescriptors: 0
6+
; CHECK-NOT: Root Signature Definitions
7+
8+
define void @main() #0 {
9+
entry:
10+
ret void
11+
}
12+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
13+
14+
15+
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
16+
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
17+
!3 = !{ !5 } ; list of root signature elements
18+
!5 = !{ !"DescriptorTable", i32 0, !6}
19+
!6 = !{ !"SRV", i32 0, i32 0, i32 10, i32 -1, i32 4 }

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1616
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
1717
!3 = !{ !5 } ; list of root signature elements
1818
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
19-
!6 = !{ !"Invalid", i32 0, i32 0, i32 -1, i32 -1, i32 4 }
19+
!6 = !{ !"Invalid", i32 1, i32 0, i32 -1, i32 -1, i32 4 }
2020
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1616
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
1717
!3 = !{ !5 } ; list of root signature elements
1818
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
19-
!6 = !{ !"SRV", i32 0, i32 0, i32 10, i32 -1, i32 4 }
19+
!6 = !{ !"SRV", i32 1, i32 0, i32 10, i32 -1, i32 4 }
2020
!7 = !{ !"UAV", i32 5, i32 1, i32 4294967280, i32 5, i32 2 }

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1616
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
1717
!3 = !{ !5 } ; list of root signature elements
1818
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
19-
!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
19+
!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
2020
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
2121

2222
; DXC: - Name: RTS0
@@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
3535
; DXC-NEXT: RangesOffset: 44
3636
; DXC-NEXT: Ranges:
3737
; DXC-NEXT: - RangeType: 0
38-
; DXC-NEXT: NumDescriptors: 0
38+
; DXC-NEXT: NumDescriptors: 1
3939
; DXC-NEXT: BaseShaderRegister: 1
4040
; DXC-NEXT: RegisterSpace: 0
4141
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
1515
!3 = !{ !4, !5, !6, !7 } ; list of root signature elements
1616
!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout
1717
!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 }
18-
!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 6 }
18+
!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 4 }
1919
!7 = !{ !"DescriptorTable", i32 0, !8, !9 }
20-
!8 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
20+
!8 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
2121
!9 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
2222

2323
;CHECK-LABEL: Definition for 'main':
@@ -34,14 +34,14 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
3434
;CHECK-NEXT: Shader Visibility: 1
3535
;CHECK-NEXT: Register Space: 5
3636
;CHECK-NEXT: Shader Register: 4
37-
;CHECK-NEXT: Flags: 6
37+
;CHECK-NEXT: Flags: 4
3838
;CHECK-NEXT: - Parameter Type: 0
3939
;CHECK-NEXT: Shader Visibility: 0
4040
;CHECK-NEXT: NumRanges: 2
4141
;CHECK-NEXT: - Range Type: 0
4242
;CHECK-NEXT: Register Space: 0
4343
;CHECK-NEXT: Base Shader Register: 1
44-
;CHECK-NEXT: Num Descriptors: 0
44+
;CHECK-NEXT: Num Descriptors: 1
4545
;CHECK-NEXT: Offset In Descriptors From Table Start: 4294967295
4646
;CHECK-NEXT: Flags: 4
4747
;CHECK-NEXT: - Range Type: 1

0 commit comments

Comments
 (0)