Skip to content

[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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented Jul 4, 2025

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
  • Add test to demonstrate updated flag verifications
  • Adds verifyNumDescriptors to the validation of DescriptorRanges
  • Add a test to demonstrate numDescriptors verification
  • Updates a number of tests that mistakenly had an invalid numDescriptors specified

Resolves: #147107

@llvmbot llvmbot added backend:DirectX HLSL HLSL Language Support labels Jul 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 4, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-backend-directx

Author: Finn Plummer (inbelic)

Changes

This pr resolves some discrepancies in verification during validate in DXILRootSignature.cpp.

Namely,

Resolves: #147107


Full diff: https://github.com/llvm/llvm-project/pull/147111.diff

10 Files Affected:

  • (modified) llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h (+2-1)
  • (modified) llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp (+22-1)
  • (modified) llvm/lib/Target/DirectX/DXILRootSignature.cpp (+5-1)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll (+2-2)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-Flag.ll (+1-1)
  • (added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-NumDescriptors.ll (+19)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RangeType.ll (+1-1)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-Invalid-RegisterSpace.ll (+1-1)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll (+2-2)
  • (modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll (+2-2)
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

@inbelic inbelic force-pushed the inbelic/rs-fix-validations branch from b6ac185 to c762e0a Compare July 4, 2025 21:48
@inbelic inbelic linked an issue Jul 4, 2025 that may be closed by this pull request
2 tasks
@inbelic inbelic changed the base branch from users/inbelic/pr-147110 to main July 8, 2025 20:38
@inbelic inbelic force-pushed the inbelic/rs-fix-validations branch from c762e0a to 22a8bf6 Compare July 8, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:DirectX HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DirectX] Missing verifications in validate of DXILRootSignature.cpp
3 participants