-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[DirectX] Add missing verifications during validate
of DXILRootSignature
#147111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-backend-directx Author: Finn Plummer (inbelic) ChangesThis pr resolves some discrepancies in verification during Namely,
Resolves: #147107 Full diff: https://github.com/llvm/llvm-project/pull/147111.diff 10 Files Affected:
diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h
index 9b68a524432cc..03f4e2cf5bb8f 100644
--- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h
+++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h
@@ -27,10 +27,11 @@ bool verifyRootFlag(uint32_t Flags);
bool verifyVersion(uint32_t Version);
bool verifyRegisterValue(uint32_t RegisterValue);
bool verifyRegisterSpace(uint32_t RegisterSpace);
-bool verifyDescriptorFlag(uint32_t Flags);
+bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal);
bool verifyRangeType(uint32_t Type);
bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
uint32_t FlagsVal);
+bool verifyNumDescriptors(uint32_t NumDescriptors);
bool verifySamplerFilter(uint32_t Value);
bool verifyAddress(uint32_t Address);
bool verifyMipLODBias(float MipLODBias);
diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp
index b5b5fc0c74d83..48d6b8639b2dc 100644
--- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp
+++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp
@@ -32,7 +32,24 @@ bool verifyRegisterSpace(uint32_t RegisterSpace) {
return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF);
}
-bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; }
+bool verifyRootDescriptorFlag(uint32_t Version, uint32_t FlagsVal) {
+ using FlagT = dxbc::RootDescriptorFlags;
+ FlagT Flags = FlagT(FlagsVal);
+ if (Version == 1)
+ return FlagsVal == FlagT::DataVolatile;
+
+ assert(Version == 2 && "Provided invalid root signature version");
+
+ // The data-specific flags are mutually exclusive.
+ FlagT DataFlags = FlagT::DataVolatile | FlagT::DataStatic |
+ FlagT::DataStaticWhileSetAtExecute;
+
+ if (popcount(llvm::to_underlying(Flags & DataFlags)) > 1)
+ return false;
+
+ // Only a data flag or no flags is valid
+ return (FlagsVal & ~0xE) == 0;
+}
bool verifyRangeType(uint32_t Type) {
switch (Type) {
@@ -108,6 +125,10 @@ bool verifyDescriptorRangeFlag(uint32_t Version, uint32_t Type,
return (Flags & ~Mask) == FlagT::None;
}
+bool verifyNumDescriptors(uint32_t NumDescriptors) {
+ return NumDescriptors > 0;
+}
+
bool verifySamplerFilter(uint32_t Value) {
switch (Value) {
#define FILTER(Num, Val) case llvm::to_underlying(dxbc::SamplerFilter::Val):
diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index e46b184a353f1..37a60075514ef 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -432,7 +432,8 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
return reportValueError(Ctx, "RegisterSpace", Descriptor.RegisterSpace);
if (RSD.Version > 1) {
- if (!llvm::hlsl::rootsig::verifyDescriptorFlag(Descriptor.Flags))
+ if (!llvm::hlsl::rootsig::verifyRootDescriptorFlag(RSD.Version,
+ Descriptor.Flags))
return reportValueError(Ctx, "DescriptorRangeFlag", Descriptor.Flags);
}
break;
@@ -447,6 +448,9 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
if (!llvm::hlsl::rootsig::verifyRegisterSpace(Range.RegisterSpace))
return reportValueError(Ctx, "RegisterSpace", Range.RegisterSpace);
+ if (!llvm::hlsl::rootsig::verifyNumDescriptors(Range.NumDescriptors))
+ return reportValueError(Ctx, "NumDescriptors", Range.NumDescriptors);
+
if (!llvm::hlsl::rootsig::verifyDescriptorRangeFlag(
RSD.Version, Range.RangeType, Range.Flags))
return reportValueError(Ctx, "DescriptorFlag", Range.Flags);
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
index 9d89dbdd9107b..053721de1eb1f 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
@@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!2 = !{ ptr @main, !3, i32 1 } ; function, root signature
!3 = !{ !5 } ; list of root signature elements
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 }
+!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
@@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: RangesOffset: 44
; DXC-NEXT: Ranges:
; DXC-NEXT: - RangeType: 3
-; DXC-NEXT: NumDescriptors: 0
+; DXC-NEXT: NumDescriptors: 1
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 0
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll
index 41101c1f1fe8e..58056c5d4ba94 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
!3 = !{ !5 } ; list of root signature elements
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 22 }
+!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 22 }
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-NumDescriptors.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-NumDescriptors.ll
new file mode 100644
index 0000000000000..99d126ee95443
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-NumDescriptors.ll
@@ -0,0 +1,19 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+; CHECK: error: Invalid value for NumDescriptors: 0
+; CHECK-NOT: Root Signature Definitions
+
+define void @main() #0 {
+entry:
+ ret void
+}
+attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
+
+
+!dx.rootsignatures = !{!2} ; list of function/root signature pairs
+!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
+!3 = !{ !5 } ; list of root signature elements
+!5 = !{ !"DescriptorTable", i32 0, !6}
+!6 = !{ !"SRV", i32 0, i32 0, i32 10, i32 -1, i32 4 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll
index b7e99ae7cd27b..0f7116307c315 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
!3 = !{ !5 } ; list of root signature elements
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"Invalid", i32 0, i32 0, i32 -1, i32 -1, i32 4 }
+!6 = !{ !"Invalid", i32 1, i32 0, i32 -1, i32 -1, i32 4 }
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll
index 4cef5d86a980b..94cd25b06a127 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll
@@ -16,5 +16,5 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
!3 = !{ !5 } ; list of root signature elements
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 0, i32 10, i32 -1, i32 4 }
+!6 = !{ !"SRV", i32 1, i32 0, i32 10, i32 -1, i32 4 }
!7 = !{ !"UAV", i32 5, i32 1, i32 4294967280, i32 5, i32 2 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
index b516d66180247..8e9b4b43b11a6 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
@@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!2 = !{ ptr @main, !3, i32 2 } ; function, root signature
!3 = !{ !5 } ; list of root signature elements
!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
+!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
!7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
; DXC: - Name: RTS0
@@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
; DXC-NEXT: RangesOffset: 44
; DXC-NEXT: Ranges:
; DXC-NEXT: - RangeType: 0
-; DXC-NEXT: NumDescriptors: 0
+; DXC-NEXT: NumDescriptors: 1
; DXC-NEXT: BaseShaderRegister: 1
; DXC-NEXT: RegisterSpace: 0
; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll
index d0a58bc34ffa4..a8444c6079a85 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll
@@ -17,7 +17,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 }
!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 6 }
!7 = !{ !"DescriptorTable", i32 0, !8, !9 }
-!8 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
+!8 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
!9 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
;CHECK-LABEL: Definition for 'main':
@@ -41,7 +41,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
;CHECK-NEXT: - Range Type: 0
;CHECK-NEXT: Register Space: 0
;CHECK-NEXT: Base Shader Register: 1
-;CHECK-NEXT: Num Descriptors: 0
+;CHECK-NEXT: Num Descriptors: 1
;CHECK-NEXT: Offset In Descriptors From Table Start: 4294967295
;CHECK-NEXT: Flags: 4
;CHECK-NEXT: - Range Type: 1
|
b6ac185
to
c762e0a
Compare
c762e0a
to
22a8bf6
Compare
This pr resolves some discrepancies in verification during
validate
inDXILRootSignature.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.
verifyDescriptorFlag
to check for valid flags based on version, as reflected hereverifyNumDescriptors
to the validation ofDescriptorRange
snumDescriptors
verificationnumDescriptors
specifiedResolves: #147107