[DirectX] Add Range Overlap validation #152229

joaosaffran · 2025-08-06T00:36:22Z

As part of the Root Signature Spec, we need to validate if Root Signatures are not defining overlapping ranges.
Closes: #126645

refactoring clean up format formating fix import issues formating refactoring init refactoring adding validation clean implementing finish implementing && fix tests sync parent sync parent address comments fix test fix tests format address changes add preserved addressing comments updating format adding tests clean up address comments adding root constants clean moving code arround clean addressing comments address comments update code cleanup address comments from inbelic

llvmbot · 2025-08-06T00:36:52Z

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-directx

Author: None (joaosaffran)

Changes

As part of the Root Signature Spec, we need to validate if Root Signatures are not defining overlapping ranges.
Closes: #126645

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

11 Files Affected:

(modified) llvm/include/llvm/Analysis/DXILResource.h (+14)
(modified) llvm/lib/Analysis/DXILResource.cpp (-14)
(modified) llvm/lib/Target/DirectX/DXILOpLowering.cpp (+2)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp (+174-3)
(modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1-1)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation-constants.ll (+22)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation-fail-cbuffer-range.ll (+15)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation-fail-descriptor-table-range.ll (+15)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation-fail-root-descriptor-range.ll (+15)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation-fail-static-sampler-range.ll (+13)
(added) llvm/test/CodeGen/DirectX/rootsignature-validation.ll (+39)

diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index 93c6bfb057ef5..96d631698af0a 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -33,6 +33,20 @@ class DXILResourceTypeMap;
 
 namespace dxil {
 
+inline StringRef getResourceClassName(ResourceClass RC) {
+  switch (RC) {
+  case ResourceClass::SRV:
+    return "SRV";
+  case ResourceClass::UAV:
+    return "UAV";
+  case ResourceClass::CBuffer:
+    return "CBuffer";
+  case ResourceClass::Sampler:
+    return "Sampler";
+  }
+  llvm_unreachable("Unhandled ResourceClass");
+}
+
 // Returns the resource name from dx_resource_handlefrombinding or
 // dx_resource_handlefromimplicitbinding call
 LLVM_ABI StringRef getResourceNameFromBindingCall(CallInst *CI);
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 629fa7cddb9d4..b476026fcc718 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -29,20 +29,6 @@
 using namespace llvm;
 using namespace dxil;
 
-static StringRef getResourceClassName(ResourceClass RC) {
-  switch (RC) {
-  case ResourceClass::SRV:
-    return "SRV";
-  case ResourceClass::UAV:
-    return "UAV";
-  case ResourceClass::CBuffer:
-    return "CBuffer";
-  case ResourceClass::Sampler:
-    return "Sampler";
-  }
-  llvm_unreachable("Unhandled ResourceClass");
-}
-
 static StringRef getResourceKindName(ResourceKind RK) {
   switch (RK) {
   case ResourceKind::Texture1D:
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 0ec15a629d0a2..db7719e129ad0 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -9,6 +9,7 @@
 #include "DXILOpLowering.h"
 #include "DXILConstants.h"
 #include "DXILOpBuilder.h"
+#include "DXILRootSignature.h"
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
 #include "llvm/ADT/SmallVector.h"
@@ -945,6 +946,7 @@ class DXILOpLoweringLegacy : public ModulePass {
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();
   }
 };
 char DXILOpLoweringLegacy::ID = 0;
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
index 398dcbb8d1737..3da4cfbea23b8 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DXILPostOptimizationValidation.h"
+#include "DXILRootSignature.h"
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
 #include "llvm/ADT/SmallString.h"
@@ -24,6 +25,35 @@ using namespace llvm;
 using namespace llvm::dxil;
 
 namespace {
+static ResourceClass RangeToResourceClass(uint32_t RangeType) {
+  using namespace dxbc;
+  switch (static_cast<DescriptorRangeType>(RangeType)) {
+  case DescriptorRangeType::SRV:
+    return ResourceClass::SRV;
+  case DescriptorRangeType::UAV:
+    return ResourceClass::UAV;
+  case DescriptorRangeType::CBV:
+    return ResourceClass::CBuffer;
+  case DescriptorRangeType::Sampler:
+    return ResourceClass::Sampler;
+  }
+}
+
+ResourceClass ParameterToResourceClass(uint32_t Type) {
+  using namespace dxbc;
+  switch (Type) {
+  case llvm::to_underlying(RootParameterType::Constants32Bit):
+    return ResourceClass::CBuffer;
+  case llvm::to_underlying(RootParameterType::SRV):
+    return ResourceClass::SRV;
+  case llvm::to_underlying(RootParameterType::UAV):
+    return ResourceClass::UAV;
+  case llvm::to_underlying(RootParameterType::CBV):
+    return ResourceClass::CBuffer;
+  default:
+    llvm_unreachable("Unknown RootParameterType");
+  }
+}
 
 static void reportInvalidDirection(Module &M, DXILResourceMap &DRM) {
   for (const auto &UAV : DRM.uavs()) {
@@ -84,8 +114,122 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
   }
 }
 
+static void reportOverlappingRegisters(
+    Module &M, const llvm::hlsl::BindingInfoBuilder::Binding &Reported,
+    const llvm::hlsl::BindingInfoBuilder::Binding &Overlaping) {
+  SmallString<128> Message;
+  raw_svector_ostream OS(Message);
+  OS << "register " << getResourceClassName(Reported.RC)
+     << " (space=" << Reported.Space << ", register=" << Reported.LowerBound
+     << ")" << " is overlapping with" << " register "
+     << getResourceClassName(Overlaping.RC) << " (space=" << Overlaping.Space
+     << ", register=" << Overlaping.LowerBound << ")"
+     << ", verify your root signature definition.";
+
+  M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static dxbc::ShaderVisibility
+tripleToVisibility(llvm::Triple::EnvironmentType ET) {
+  assert((ET == Triple::Pixel || ET == Triple::Vertex ||
+          ET == Triple::Geometry || ET == Triple::Hull ||
+          ET == Triple::Domain || ET == Triple::Mesh ||
+          ET == Triple::Compute) &&
+         "Invalid Triple to shader stage conversion");
+
+  switch (ET) {
+  case Triple::Pixel:
+    return dxbc::ShaderVisibility::Pixel;
+  case Triple::Vertex:
+    return dxbc::ShaderVisibility::Vertex;
+  case Triple::Geometry:
+    return dxbc::ShaderVisibility::Geometry;
+  case Triple::Hull:
+    return dxbc::ShaderVisibility::Hull;
+  case Triple::Domain:
+    return dxbc::ShaderVisibility::Domain;
+  case Triple::Mesh:
+    return dxbc::ShaderVisibility::Mesh;
+  case Triple::Compute:
+    return dxbc::ShaderVisibility::All;
+  default:
+    llvm_unreachable("Invalid triple to shader stage conversion");
+  }
+}
+
+static void trackRootSigDescBinding(hlsl::BindingInfoBuilder &Builder,
+                                    const mcdxbc::RootSignatureDesc &RSD,
+                                    dxbc::ShaderVisibility Visibility) {
+  for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+    const auto &[Type, Loc] =
+        RSD.ParametersContainer.getTypeAndLocForParameter(I);
+
+    const auto &Header = RSD.ParametersContainer.getHeader(I);
+    if (Header.ShaderVisibility !=
+            llvm::to_underlying(dxbc::ShaderVisibility::All) &&
+        Header.ShaderVisibility != llvm::to_underlying(Visibility))
+      continue;
+
+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {
+      dxbc::RTS0::v1::RootConstants Const =
+          RSD.ParametersContainer.getConstant(Loc);
+      Builder.trackBinding(dxil::ResourceClass::CBuffer, Const.RegisterSpace,
+                           Const.ShaderRegister,
+                           Const.ShaderRegister + Const.Num32BitValues, &Const);
+      break;
+    }
+
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+    case llvm::to_underlying(dxbc::RootParameterType::CBV): {
+      dxbc::RTS0::v2::RootDescriptor Desc =
+          RSD.ParametersContainer.getRootDescriptor(Loc);
+      Builder.trackBinding(ParameterToResourceClass(Type), Desc.RegisterSpace,
+                           Desc.ShaderRegister, Desc.ShaderRegister, &Desc);
+
+      break;
+    }
+    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+      const mcdxbc::DescriptorTable &Table =
+          RSD.ParametersContainer.getDescriptorTable(Loc);
+
+      for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+        Builder.trackBinding(RangeToResourceClass(Range.RangeType),
+                             Range.RegisterSpace, Range.BaseShaderRegister,
+                             Range.NumDescriptors == ~0U
+                                 ? Range.NumDescriptors
+                                 : Range.BaseShaderRegister +
+                                       Range.NumDescriptors,
+                             &Range);
+      }
+      break;
+    }
+    }
+  }
+
+  for (auto &S : RSD.StaticSamplers) {
+    Builder.trackBinding(dxil::ResourceClass::Sampler, S.RegisterSpace,
+                         S.ShaderRegister, S.ShaderRegister, &S);
+  }
+}
+
+std::optional<mcdxbc::RootSignatureDesc>
+getRootSignature(RootSignatureBindingInfo &RSBI,
+                 dxil::ModuleMetadataInfo &MMI) {
+  if (MMI.EntryPropertyVec.size() == 0)
+    return std::nullopt;
+  std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
+      RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);
+  if (!RootSigDesc)
+    return std::nullopt;
+  return RootSigDesc;
+}
+
 static void reportErrors(Module &M, DXILResourceMap &DRM,
-                         DXILResourceBindingInfo &DRBI) {
+                         DXILResourceBindingInfo &DRBI,
+                         RootSignatureBindingInfo &RSBI,
+                         dxil::ModuleMetadataInfo &MMI) {
   if (DRM.hasInvalidCounterDirection())
     reportInvalidDirection(M, DRM);
 
@@ -94,6 +238,20 @@ static void reportErrors(Module &M, DXILResourceMap &DRM,
 
   assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in "
                                        "DXILResourceImplicitBinding pass");
+
+  if (auto RSD = getRootSignature(RSBI, MMI)) {
+
+    hlsl::BindingInfoBuilder Builder;
+    dxbc::ShaderVisibility Visibility = tripleToVisibility(MMI.ShaderProfile);
+    trackRootSigDescBinding(Builder, *RSD, Visibility);
+    hlsl::BindingInfo Info = Builder.calculateBindingInfo(
+        [&M](const llvm::hlsl::BindingInfoBuilder &Builder,
+             const llvm::hlsl::BindingInfoBuilder::Binding &ReportedBinding) {
+          const llvm::hlsl::BindingInfoBuilder::Binding &Overlaping =
+              Builder.findOverlapping(ReportedBinding);
+          reportOverlappingRegisters(M, ReportedBinding, Overlaping);
+        });
+  }
 }
 } // namespace
 
@@ -101,7 +259,10 @@ PreservedAnalyses
 DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILResourceMap &DRM = MAM.getResult<DXILResourceAnalysis>(M);
   DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M);
-  reportErrors(M, DRM, DRBI);
+  RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M);
+  ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M);
+
+  reportErrors(M, DRM, DRBI, RSBI, MMI);
   return PreservedAnalyses::all();
 }
 
@@ -113,7 +274,12 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
         getAnalysis<DXILResourceWrapperPass>().getResourceMap();
     DXILResourceBindingInfo &DRBI =
         getAnalysis<DXILResourceBindingWrapperPass>().getBindingInfo();
-    reportErrors(M, DRM, DRBI);
+    RootSignatureBindingInfo &RSBI =
+        getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
+    dxil::ModuleMetadataInfo &MMI =
+        getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+
+    reportErrors(M, DRM, DRBI, RSBI, MMI);
     return false;
   }
   StringRef getPassName() const override {
@@ -125,10 +291,13 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addRequired<DXILResourceWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
+    AU.addRequired<DXILMetadataAnalysisWrapperPass>();
+    AU.addRequired<RootSignatureAnalysisWrapper>();
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILResourceBindingWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();
   }
 };
 char DXILPostOptimizationValidationLegacy::ID = 0;
@@ -139,6 +308,8 @@ INITIALIZE_PASS_BEGIN(DXILPostOptimizationValidationLegacy, DEBUG_TYPE,
 INITIALIZE_PASS_DEPENDENCY(DXILResourceBindingWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DXILResourceTypeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DXILResourceWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(DXILMetadataAnalysisWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(RootSignatureAnalysisWrapper)
 INITIALIZE_PASS_END(DXILPostOptimizationValidationLegacy, DEBUG_TYPE,
                     "DXIL Post Optimization Validation", false, false)
 
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index 151603a7161c5..c31d2194a8de5 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -33,9 +33,9 @@
 ; CHECK-NEXT:   DXIL Module Metadata analysis
 ; CHECK-NEXT:   DXIL Shader Flag Analysis
 ; CHECK-NEXT:   DXIL Translate Metadata
+; CHECK-NEXT:   DXIL Root Signature Analysis
 ; CHECK-NEXT:   DXIL Post Optimization Validation
 ; CHECK-NEXT:   DXIL Op Lowering
-; CHECK-NEXT:   DXIL Root Signature Analysis
 ; CHECK-NEXT:   DXIL Prepare Module
 
 ; CHECK-ASM-NEXT: DXIL Metadata Pretty Printer
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-constants.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-constants.ll
new file mode 100644
index 0000000000000..3c33f1dc6bdf0
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-constants.ll
@@ -0,0 +1,22 @@
+; RUN: opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 
+; expected-no-diagnostics
+; Root Signature(RootConstants(num32BitConstants=4, b2))
+
+%__cblayout_CB = type <{ float }>
+
+@CB.str = private unnamed_addr constant [3 x i8] c"CB\00", align 1
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+; cbuffer CB : register(b2, space0) {
+;  float a;
+; }
+  %CB = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 4, 0)) @llvm.dx.resource.handlefrombinding(i32 0, i32 2, i32 1, i32 0, i1 false, ptr nonnull @CB.str)
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2}
+!2 = !{!"RootConstants", i32 0, i32 2, i32 0, i32 4}
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-cbuffer-range.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-cbuffer-range.ll
new file mode 100644
index 0000000000000..47d2a5b8d5aab
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-cbuffer-range.ll
@@ -0,0 +1,15 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: register CBuffer (space=0, register=2) is overlapping with register CBuffer (space=0, register=0), verify your root signature definition.
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+; RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3))
+!dx.rootsignatures = !{!0}
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2, !3}
+!2 = !{!"RootConstants", i32 0, i32 2, i32 0, i32 4}
+!3 = !{!"DescriptorTable", i32 0, !4}
+!4 = !{!"CBV", i32 3, i32 0, i32 0, i32 -1, i32 4}
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-descriptor-table-range.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-descriptor-table-range.ll
new file mode 100644
index 0000000000000..dbc75ac86db5f
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-descriptor-table-range.ll
@@ -0,0 +1,15 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: register UAV (space=10, register=4294967295) is overlapping with register UAV (space=10, register=4294967295), verify your root signature definition.
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+; DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), DescriptorTable(UAV(u2, numDescriptors=4))
+!dx.rootsignatures = !{!0}
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2, !4}
+!2 = !{!"DescriptorTable", i32 0, !3}
+!3 = !{!"UAV", i32 -1, i32 -1, i32 10, i32 -1, i32 2}
+!4 = !{!"DescriptorTable", i32 0, !5}
+!5 = !{ !"UAV", i32 -1, i32 -1, i32 10, i32 5, i32 2 }
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-root-descriptor-range.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-root-descriptor-range.ll
new file mode 100644
index 0000000000000..9f70cc3f062a9
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-root-descriptor-range.ll
@@ -0,0 +1,15 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: register UAV (space=1, register=3) is overlapping with register UAV (space=1, register=0), verify your root signature definition.
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+; DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), UAV(u3, space=1)
+!dx.rootsignatures = !{!0}
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2, !4}
+!2 = !{!"RootUAV", i32 0, i32 3, i32 1, i32 4}
+!4 = !{!"DescriptorTable", i32 0, !5}
+!5 = !{!"UAV", i32 3, i32 0, i32 1, i32 -1, i32 2}
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-static-sampler-range.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-static-sampler-range.ll
new file mode 100644
index 0000000000000..340b0aed6d16b
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-static-sampler-range.ll
@@ -0,0 +1,13 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: register Sampler (space=0, register=42) is overlapping with register Sampler (space=0, register=42), verify your root signature definition.
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2, !3}
+!2 = !{ !"StaticSampler", i32 5, i32 4, i32 5, i32 3, float 0x3FF7CCCCC0000000, i32 10, i32 2, i32 1, float -1.270000e+02, float 1.220000e+02, i32 42, i32 0, i32 0 }
+!3 = !{ !"StaticSampler", i32 4, i32 2, i32 3, i32 5, float 0x3FF6CCCCC0000000, i32 9, i32 3, i32 2, float -1.280000e+02, float 1.280000e+02, i32 42, i32 0, i32 0 }
diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation.ll
new file mode 100644
index 0000000000000..750679bf743c5
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/rootsignature-validation.ll
@@ -0,0 +1,39 @@
+; RUN: opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 
+; expected-no-diagnostics
+
+
+; Root Signature(
+;   CBV(b3, space=1, visibility=SHADER_VISIBILITY_ALL)
+;   DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_ALL)
+;   DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_VERTEX)
+;   DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)
+
+%__cblayout_CB = type <{ float }>
+
+@CB.str = private unnamed_addr constant [3 x i8] c"CB\00", align 1
+@Smp.str = private unnamed_addr constant [4 x i8] c"Smp\00", align 1
+@SB.str = private unnamed_addr constant [3 x i8] c"SB\00", align 1
+@RWB.str = private unnamed_addr constant [4 x i8] c"RWB\00", align 1
+
+define void @CSMain() "hlsl.shader"="compute" {
+entry:
+
+  %CB = tail call target("dx.CBuffer", target("dx.Layout", %__cblayout_CB, 4, 0)) @llvm.dx.resource.handlefrombinding(i32 1, i32 3, i32 1, i32 0, i1 false, ptr nonnull @CB.str)
+  %Sampler = call target("dx.Sampler", 0) @llvm.dx.resource.handlefrombinding(i32 2, i32 3, i32 1, i32 0, i1 false, ptr nonnull @Smp.str)
+  %SB = tail call target("dx.RawBuffer", i32, 0, 0) @llvm.dx.resource.handlefrombinding.tdx.RawBuffer_i32_0_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @SB.str)
+  %RWB =  tail call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0_0t(i32 0, i32 0, i32 1, i32 0, i1 false, ptr nonnull @RWB.str)
+
+  ret void
+}
+
+!dx.rootsignatures = !{!0}
+
+!0 = !{ptr @CSMain, !1, i32 2}
+!1 = !{!2, !3, !5, !7}
+!2 = !{!"RootCBV", i32 0, i32 3, i32 1, i32 4}
+!3 = !{!"DescriptorTable", i32 0, !4}
+!4 = !{!"SRV", i32 1, i32 0, i32 0, i32 -1, i32 0}
+!5 = !{!"DescriptorTable", i32 0, !6}
+!6 = !{!"Sampler", i32 5, i32 3, i32 2, i32 -1, i32 0}
+!7 = !{!"DescriptorTable", i32 0, !8}
+!8 = !{!"UAV", i32 -1, i32 0, i32 0, i32 -1, i32 2}

github-actions · 2025-08-06T00:38:02Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

inbelic · 2025-08-06T15:34:33Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+                                 : Range.BaseShaderRegister +
+                                       Range.NumDescriptors,


Suggested change

: Range.BaseShaderRegister +

Range.NumDescriptors,

: Range.BaseShaderRegister +

Range.NumDescriptors - 1,

I believe this is an inclusive range. We should add a test case for this case.

Probably more readable to introduce a UpperBound variable rather than bury the ternary operator in the function call as well.

inbelic · 2025-08-06T15:37:29Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+          RSD.ParametersContainer.getConstant(Loc);
+      Builder.trackBinding(dxil::ResourceClass::CBuffer, Const.RegisterSpace,
+                           Const.ShaderRegister,
+                           Const.ShaderRegister + Const.Num32BitValues, &Const);


Suggested change

Const.ShaderRegister + Const.Num32BitValues, &Const);

Const.ShaderRegister, &Const);

This is what we do in the frontend, and iirc, I did check the behaviour with DXC. In either case, can you double-check what the correct behaviour is. But I believe it will always have a size of 1.

inbelic · 2025-08-06T15:39:05Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+  for (auto &S : RSD.StaticSamplers) {
+    Builder.trackBinding(dxil::ResourceClass::Sampler, S.RegisterSpace,
+                         S.ShaderRegister, S.ShaderRegister, &S);
+  }


nit:

Suggested change

for (auto &S : RSD.StaticSamplers) {

Builder.trackBinding(dxil::ResourceClass::Sampler, S.RegisterSpace,

S.ShaderRegister, S.ShaderRegister, &S);

}

for (auto &S : RSD.StaticSamplers)

Builder.trackBinding(dxil::ResourceClass::Sampler, S.RegisterSpace,

S.ShaderRegister, S.ShaderRegister, &S);

inbelic · 2025-08-06T15:42:21Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

    AU.addPreserved<DXILResourceWrapperPass>();
    AU.addPreserved<DXILResourceBindingWrapperPass>();
    AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
    AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();


What do addRequired and addPreserved do? Not saying anything is wrong, I just don't understand.

addRequired adds another analysis as a dependency for you current analysis, this way the pass manager can run the required ones before your analysis.

addPreserved tells the pass manager to preserve the result of another pass. This is most useful in cases where the pass changes the IR, and you don't want to invalidate another analysis.

Cool thanks

bogner

We probably need some more tests here, but it looks like it's moving in the right direction.

Please run the offload-test-suite for this kind of change - trying it locally I see a lot of failures. Most of them look like they're the same issue as #152250, but even after cherry picking that change there are a number of failures.

bogner · 2025-08-06T16:46:25Z

llvm/lib/Target/DirectX/DXILOpLowering.cpp

@@ -945,6 +946,7 @@ class DXILOpLoweringLegacy : public ModulePass {
    AU.addPreserved<DXILResourceWrapperPass>();
    AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
    AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();


We'll need to preserve the root signature analysis in the new-PM version of the pass as well (that is, in DXILOpLowering::run)

bogner · 2025-08-06T23:23:00Z

llvm/include/llvm/Analysis/DXILResource.h

@@ -33,6 +33,20 @@ class DXILResourceTypeMap;

 namespace dxil {

+inline StringRef getResourceClassName(ResourceClass RC) {


This isn't an appropriate place for this function - the enum is defined in Support/DXILABI.h and this is just an arbitrary user of that that happens to also be a dependency of where you're using this functionality. Also, it's not a "great" practice to have a bunch of string literals in an inline function in a header - these will end up as duplicate data in every translation unit and we have to rely on the linker to clean it all up later.

I guess it would probably be best to add a DXILABI.cpp in Support and out-of-line the resource class enum stringify-ing function there.

bogner · 2025-08-06T23:25:22Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+static ResourceClass RangeToResourceClass(uint32_t RangeType) {
+  using namespace dxbc;
+  switch (static_cast<DescriptorRangeType>(RangeType)) {


It's weird for this function to take a uint32_t, and this function isn't following llvm naming conventions. We can solve both of these by creating a strongly typed conversion function that can be overloaded, like static ResourceClass toResourceClass(dxbc::DescriptorRangeType).

bogner · 2025-08-06T23:26:26Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

@@ -24,6 +25,35 @@ using namespace llvm;
 using namespace llvm::dxil;

 namespace {


This is already there, but static functions don't need to be in an anonymous namespace (only classes and structs do). I guess we could remove this particular anonymous namespace completely.

bogner · 2025-08-06T23:26:58Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+  }
+}
+
+ResourceClass ParameterToResourceClass(uint32_t Type) {


Similarly, toResourceClass(dxbc::RootParameterType) would be better.

bogner · 2025-08-06T23:41:51Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {


Similarly here - Using the underlying type for the switch is needlessly confusing and defeats covered enum warnings. Why not:

dxbc::RootParameterType ParamType = static_cast<dxbc::RootParameterType>(ParamInfo.Header.ParameterType); switch (ParamType) { case dxbc::RootParameterType::Constants32Bit: { // ...

bogner · 2025-08-06T23:42:57Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+          RSD.ParametersContainer.getConstant(Loc);
+      Builder.trackBinding(dxil::ResourceClass::CBuffer, Const.RegisterSpace,
+                           Const.ShaderRegister,
+                           Const.ShaderRegister + Const.Num32BitValues, &Const);


&Const is a reference to a local here - this is undefined behaviour. If we need a cookie to deduplicate these bindings we'll need something that has a sufficient lifetime. Same with the following calls to trackBinding

bogner · 2025-08-06T23:44:07Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+                                 : Range.BaseShaderRegister +
+                                       Range.NumDescriptors,


Probably more readable to introduce a UpperBound variable rather than bury the ternary operator in the function call as well.

bogner · 2025-08-06T23:45:43Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+    }
+  }
+
+  for (auto &S : RSD.StaticSamplers) {


It's best to only use auto when the type is blatantly obvious like a cast, or sometimes in cases like iterators where the type name is long enough to obscure what's happening. Please spell out the type here. Also, should this be const?

bogner · 2025-08-06T23:49:36Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+  std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
+      RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);


I'm a bit surprised / worried that this API is returning a std::optional<mcdxbc::RootSignatureDesc> and not a std::optional<mcdxbc::RootSignatureDesc &> - the root signature description object doesn't look cheap to copy (it has multiple vectors in it!) and here we are copying it twice...

std::optional don't seem to support reference, used a pointer instead.

inbelic · 2025-08-08T16:22:34Z

llvm/include/llvm/Support/DXILABI.h

@@ -99,6 +100,8 @@ enum class SamplerFeedbackType : uint32_t {
 const unsigned MinWaveSize = 4;
 const unsigned MaxWaveSize = 128;

+StringRef getResourceClassName(ResourceClass RC);


You should be able to use the recently landed function defined here in tangent with enumToStringRef: #152213

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

inbelic · 2025-08-08T16:29:22Z

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-root-descriptor-range.ll

@@ -0,0 +1,14 @@
+; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; CHECK: error: register UAV (space=1, register=3) is overlapping with register UAV (space=1, register=0), verify your root signature definition.


This error message would seemingly indicate that they shouldn't have overlapped? Maybe something like:
resource UAV(space=1, registers=[3,3]) is overlapping with resource UAV (space=1,registers=[0,3]), ...?

joaosaffran and others added 5 commits August 5, 2025 15:02

add tests

af6aa53

cleanup

29eb893

clean

ed4c553

clean up

28fb609

llvmbot added backend:DirectX llvm:analysis Includes value tracking, cost tables and constant folding labels Aug 6, 2025

inbelic self-requested a review August 6, 2025 15:26

inbelic reviewed Aug 6, 2025

View reviewed changes

bogner reviewed Aug 6, 2025

View reviewed changes

addressing comments from bogner and inbelic

403972d

llvmbot added the llvm:support label Aug 7, 2025

joaosaffran requested review from bogner and inbelic August 7, 2025 22:09

joaosaffran-zz added 3 commits August 7, 2025 15:16

clean up

0f0435d

clean up

e841a98

fix build

ae6d67a

inbelic reviewed Aug 8, 2025

View reviewed changes

joaosaffran-zz and others added 4 commits August 8, 2025 10:44

fix tests

41f32bd

Merge branch 'main' into validation/overlapping-ranges

6f3d019

Merge branch 'main' into validation/overlapping-ranges

971ad57

update

db73d71

	Const.ShaderRegister + Const.Num32BitValues, &Const);
	Const.ShaderRegister, &Const);

		@@ -33,6 +33,20 @@ class DXILResourceTypeMap;

		namespace dxil {

		inline StringRef getResourceClassName(ResourceClass RC) {

		@@ -24,6 +25,35 @@ using namespace llvm;
		using namespace llvm::dxil;

		namespace {

		switch (Type) {
		case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {

		std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
		RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);

		@@ -0,0 +1,14 @@
		; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 \| FileCheck %s
		; CHECK: error: register UAV (space=1, register=3) is overlapping with register UAV (space=1, register=0), verify your root signature definition.

[DirectX] Add Range Overlap validation #152229

Are you sure you want to change the base?

[DirectX] Add Range Overlap validation #152229

Conversation

joaosaffran commented Aug 6, 2025

Uh oh!

llvmbot commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bogner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Aug 6, 2025 •

edited

Loading