[DirectX] Add missing verifications during validate of DXILRootSignature (llvm#147111)

inbelic · web-flow · commit 420e2f584de2 · 2025-07-09T12:02:02.000-07:00
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: llvm#147107
diff --git 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
@@ -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 Flags == 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 (Flags | DataFlags) == DataFlags;
+}
 
 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
@@ -432,8 +432,9 @@ 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))
-          return reportValueError(Ctx, "DescriptorRangeFlag", Descriptor.Flags);
+        if (!llvm::hlsl::rootsig::verifyRootDescriptorFlag(RSD.Version,
+                                                           Descriptor.Flags))
+          return reportValueError(Ctx, "RootDescriptorFlag", 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -15,9 +15,9 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !3 = !{ !4, !5, !6, !7 } ; list of root signature elements
 !4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout
 !5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 }
-!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 6 }
+!6 = !{ !"RootSRV", i32 1, i32 4, i32 5, i32 4 }
 !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':
@@ -34,14 +34,14 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ;CHECK-NEXT:     Shader Visibility: 1
 ;CHECK-NEXT:     Register Space: 5
 ;CHECK-NEXT:     Shader Register: 4
-;CHECK-NEXT:     Flags: 6
+;CHECK-NEXT:     Flags: 4
 ;CHECK-NEXT:   - Parameter Type: 0
 ;CHECK-NEXT:     Shader Visibility: 0
 ;CHECK-NEXT:     NumRanges: 2
 ;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
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll
@@ -3,7 +3,7 @@
 target triple = "dxil-unknown-shadermodel6.0-compute"
 
 
-; CHECK: error: Invalid value for DescriptorRangeFlag: 3
+; CHECK: error: Invalid value for RootDescriptorFlag: 3
 ; CHECK-NOT: Root Signature Definitions
 define void @main() #0 {
 entry:
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Multiple-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Multiple-Flags.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 that 10 = DataVolatile | Datastatic is invalid due to mutually exclusive
+
+; CHECK: error: Invalid value for RootDescriptorFlag: 10
+; 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 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 10 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor_V1.ll
@@ -15,7 +15,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
-!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 8  }
+!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 2  }
 
 ; DXC:  - Name:            RTS0
 ; DXC-NEXT:    Size:            44
@@ -31,4 +31,4 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:          Descriptor:
 ; DXC-NEXT:            RegisterSpace: 2
 ; DXC-NEXT:            ShaderRegister: 1
-; DXC-NOT:            DATA_STATIC: true
+; DXC-NOT:             DATA_VOLATILE: true
