From 50e78d283fb2915a56cd3844f83d1b97a14bf2b8 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 11:21:17 -0700 Subject: [PATCH 01/18] check if table mix samplers with others --- .../DXILPostOptimizationValidation.cpp | 66 +++++++++++++++++-- ...otsignature-validation-fail-sampler-mix.ll | 18 +++++ 2 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/rootsignature-validation-fail-sampler-mix.ll diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index abcefe287d9bc..a07f92d5500fa 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -21,6 +21,7 @@ #include "llvm/InitializePasses.h" #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/Support/DXILABI.h" +#include #define DEBUG_TYPE "dxil-post-optimization-validation" @@ -155,6 +156,19 @@ reportRegNotBound(Module &M, ResourceClass Class, M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } +static void +reportDescriptorTableMixingTypes(Module &M, uint32_t Location, + dxbc::DescriptorRangeType RangeType) { + SmallString<128> Message; + raw_svector_ostream OS(Message); + OS << "Samplers cannot be mixed with other " + << "resource types in a descriptor table, " + << getResourceClassName(toResourceClass(RangeType)) + << "(location=" << Location << ")"; + + M.getContext().diagnose(DiagnosticInfoGeneric(Message)); +} + static void reportInvalidHandleTy( Module &M, const llvm::ArrayRef &RDs, const iterator_range::iterator> @@ -236,10 +250,47 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, return RDs; } -static void validateRootSignature(Module &M, - const mcdxbc::RootSignatureDesc &RSD, - dxil::ModuleMetadataInfo &MMI, - DXILResourceMap &DRM) { +static void validateDescriptorTables(Module &M, + const mcdxbc::RootSignatureDesc &RSD, + dxil::ModuleMetadataInfo &MMI, + DXILResourceMap &DRM) { + for (const mcdxbc::RootParameterInfo &ParamInfo : RSD.ParametersContainer) { + if (static_cast(ParamInfo.Header.ParameterType) != + dxbc::RootParameterType::DescriptorTable) + continue; + + mcdxbc::DescriptorTable Table = + RSD.ParametersContainer.getDescriptorTable(ParamInfo.Location); + + // Samplers cannot be mixed with other resources in a descriptor table. + bool HasSampler = false; + bool HasOtherRangeType = false; + dxbc::DescriptorRangeType OtherRangeType; + uint32_t OtherRangeTypeLocation = 0; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + dxbc::DescriptorRangeType RangeType = + static_cast(Range.RangeType); + if (RangeType == dxbc::DescriptorRangeType::Sampler) { + HasSampler = true; + } else { + HasOtherRangeType = true; + OtherRangeType = RangeType; + OtherRangeTypeLocation = ParamInfo.Location; + } + } + if (HasSampler && HasOtherRangeType) { + reportDescriptorTableMixingTypes(M, OtherRangeTypeLocation, + OtherRangeType); + continue; + } + } +} + +static void validateRootSignatureBindings(Module &M, + const mcdxbc::RootSignatureDesc &RSD, + dxil::ModuleMetadataInfo &MMI, + DXILResourceMap &DRM) { hlsl::BindingInfoBuilder Builder; dxbc::ShaderVisibility Visibility = tripleToVisibility(MMI.ShaderProfile); @@ -309,7 +360,6 @@ static void validateRootSignature(Module &M, reportOverlappingRegisters(M, ReportedBinding, Overlaping); }); // Next checks require that the root signature definition is valid. - // Next checks require that the root signature definition is valid. if (!HasOverlap) { SmallVector RDs = getRootDescriptorsBindingInfo(RSD, Visibility); @@ -351,8 +401,10 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in " "DXILResourceImplicitBinding pass"); - if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI)) - validateRootSignature(M, *RSD, MMI, DRM); + if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI)) { + validateRootSignatureBindings(M, *RSD, MMI, DRM); + validateDescriptorTables(M, *RSD, MMI, DRM); + } } PreservedAnalyses diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-sampler-mix.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-sampler-mix.ll new file mode 100644 index 0000000000000..d4caeb2675d82 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-sampler-mix.ll @@ -0,0 +1,18 @@ +; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s +; CHECK: error: Samplers cannot be mixed with other resource types in a descriptor table, UAV(location=0) + +@TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 + +define void @CSMain() "hlsl.shader"="compute" { +entry: + ret void +} + +!dx.rootsignatures = !{!0} + +!0 = !{ptr @CSMain, !1, i32 2} +!1 = !{!3} +!3 = !{!"DescriptorTable", i32 0, !4, !5} +!4 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0} +!5 = !{!"Sampler", i32 2, i32 0, i32 0, i32 -1, i32 0} + From aee4c564bcd7698c1b3574b8acf8c37ca033e52b Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 12:10:33 -0700 Subject: [PATCH 02/18] validating register not overflow in table --- .../DXILPostOptimizationValidation.cpp | 40 ++++++++++++++++++- ...ture-validation-fail-appending-overflow.ll | 18 +++++++++ ...ature-validation-fail-register-overflow.ll | 17 ++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll create mode 100644 llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index a07f92d5500fa..3721b5f539b8c 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -169,6 +169,18 @@ reportDescriptorTableMixingTypes(Module &M, uint32_t Location, M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } +static void reportOverlowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { + SmallString<128> Message; + raw_svector_ostream OS(Message); + OS << "Cannot append range with implicit lower " + << "bound after an unbounded range " + << getResourceClassName(toResourceClass(static_cast(Range.RangeType))) + << "(register=" << Range.BaseShaderRegister << ", space=" << + Range.RegisterSpace + << ") exceeds maximum allowed value."; + M.getContext().diagnose(DiagnosticInfoGeneric(Message)); +} + static void reportInvalidHandleTy( Module &M, const llvm::ArrayRef &RDs, const iterator_range::iterator> @@ -250,6 +262,8 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, return RDs; } + + static void validateDescriptorTables(Module &M, const mcdxbc::RootSignatureDesc &RSD, dxil::ModuleMetadataInfo &MMI, @@ -262,15 +276,37 @@ static void validateDescriptorTables(Module &M, mcdxbc::DescriptorTable Table = RSD.ParametersContainer.getDescriptorTable(ParamInfo.Location); - // Samplers cannot be mixed with other resources in a descriptor table. bool HasSampler = false; bool HasOtherRangeType = false; dxbc::DescriptorRangeType OtherRangeType; uint32_t OtherRangeTypeLocation = 0; + uint64_t AppendingOffset = 0; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); + + uint64_t Offset = AppendingOffset; + if(Range.OffsetInDescriptorsFromTableStart != ~0U) + Offset = Range.OffsetInDescriptorsFromTableStart; + + if(Offset > ~0U) + reportOverlowingRange(M, Range); + if(Range.NumDescriptors == ~0U) { + AppendingOffset = (uint64_t)~0U + (uint64_t)1ULL; + } else { + uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if(UpperBound > ~0U) + reportOverlowingRange(M, Range); + + uint64_t AppendingUpperBound = (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if(AppendingUpperBound > ~0U) + reportOverlowingRange(M, Range); + AppendingOffset = Offset + Range.NumDescriptors; + } + if (RangeType == dxbc::DescriptorRangeType::Sampler) { HasSampler = true; } else { @@ -279,6 +315,8 @@ static void validateDescriptorTables(Module &M, OtherRangeTypeLocation = ParamInfo.Location; } } + + // Samplers cannot be mixed with other resources in a descriptor table. if (HasSampler && HasOtherRangeType) { reportDescriptorTableMixingTypes(M, OtherRangeTypeLocation, OtherRangeType); diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll new file mode 100644 index 0000000000000..865312a43b5a8 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll @@ -0,0 +1,18 @@ +; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s +; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=3, space=4) exceeds maximum allowed value. + +@TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 + +define void @CSMain() "hlsl.shader"="compute" { +entry: + ret void +} + +!dx.rootsignatures = !{!0} + +!0 = !{ptr @CSMain, !1, i32 2} +!1 = !{!3} +!3 = !{!"DescriptorTable", i32 0, !4, !5} +!4 = !{!"UAV", i32 -1, i32 1, i32 2, i32 -1, i32 0} +!5 = !{!"UAV", i32 1, i32 3, i32 4, i32 -1, i32 0} + diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll new file mode 100644 index 0000000000000..ef887baf80728 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll @@ -0,0 +1,17 @@ +; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s +; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=4294967295, space=0) exceeds maximum allowed value. + +@TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 + +define void @CSMain() "hlsl.shader"="compute" { +entry: + ret void +} + +!dx.rootsignatures = !{!0} + +!0 = !{ptr @CSMain, !1, i32 2} +!1 = !{!3} +!3 = !{!"DescriptorTable", i32 0, !4} +!4 = !{!"UAV", i32 100, i32 4294967295, i32 0, i32 -1, i32 0} + From 7d1c13ee694848d9211c24c414bf14727a8395c7 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 13:19:37 -0700 Subject: [PATCH 03/18] clean --- llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 3721b5f539b8c..7067d6a2960e8 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -21,7 +21,6 @@ #include "llvm/InitializePasses.h" #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/Support/DXILABI.h" -#include #define DEBUG_TYPE "dxil-post-optimization-validation" From 85719afb44b59b1aa5f6afdb494983983da0ce1a Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 13:26:11 -0700 Subject: [PATCH 04/18] format --- .../DXILPostOptimizationValidation.cpp | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 7067d6a2960e8..f749e255b5125 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -168,15 +168,16 @@ reportDescriptorTableMixingTypes(Module &M, uint32_t Location, M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } -static void reportOverlowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { +static void +reportOverlowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { SmallString<128> Message; raw_svector_ostream OS(Message); - OS << "Cannot append range with implicit lower " - << "bound after an unbounded range " - << getResourceClassName(toResourceClass(static_cast(Range.RangeType))) - << "(register=" << Range.BaseShaderRegister << ", space=" << - Range.RegisterSpace - << ") exceeds maximum allowed value."; + OS << "Cannot append range with implicit lower " + << "bound after an unbounded range " + << getResourceClassName(toResourceClass( + static_cast(Range.RangeType))) + << "(register=" << Range.BaseShaderRegister + << ", space=" << Range.RegisterSpace << ") exceeds maximum allowed value."; M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } @@ -261,8 +262,6 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, return RDs; } - - static void validateDescriptorTables(Module &M, const mcdxbc::RootSignatureDesc &RSD, dxil::ModuleMetadataInfo &MMI, @@ -282,30 +281,31 @@ static void validateDescriptorTables(Module &M, uint64_t AppendingOffset = 0; - for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); - + uint64_t Offset = AppendingOffset; - if(Range.OffsetInDescriptorsFromTableStart != ~0U) + if (Range.OffsetInDescriptorsFromTableStart != ~0U) Offset = Range.OffsetInDescriptorsFromTableStart; - - if(Offset > ~0U) + + if (Offset > ~0U) reportOverlowingRange(M, Range); - if(Range.NumDescriptors == ~0U) { + if (Range.NumDescriptors == ~0U) { AppendingOffset = (uint64_t)~0U + (uint64_t)1ULL; - } else { - uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if(UpperBound > ~0U) + } else { + uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (UpperBound > ~0U) reportOverlowingRange(M, Range); - uint64_t AppendingUpperBound = (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if(AppendingUpperBound > ~0U) + uint64_t AppendingUpperBound = + (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (AppendingUpperBound > ~0U) reportOverlowingRange(M, Range); AppendingOffset = Offset + Range.NumDescriptors; } - + if (RangeType == dxbc::DescriptorRangeType::Sampler) { HasSampler = true; } else { From cc304fd464e1133cffc59cd28705cb3d68b25cbe Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 13:30:22 -0700 Subject: [PATCH 05/18] format --- llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index f749e255b5125..762a67a4c7f76 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -263,9 +263,7 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, } static void validateDescriptorTables(Module &M, - const mcdxbc::RootSignatureDesc &RSD, - dxil::ModuleMetadataInfo &MMI, - DXILResourceMap &DRM) { + const mcdxbc::RootSignatureDesc &RSD) { for (const mcdxbc::RootParameterInfo &ParamInfo : RSD.ParametersContainer) { if (static_cast(ParamInfo.Header.ParameterType) != dxbc::RootParameterType::DescriptorTable) @@ -440,7 +438,7 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI)) { validateRootSignatureBindings(M, *RSD, MMI, DRM); - validateDescriptorTables(M, *RSD, MMI, DRM); + validateDescriptorTables(M, *RSD); } } From 3c2814223ab6c439717e1a2bc15c4f929dfcb554 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 13:37:06 -0700 Subject: [PATCH 06/18] fix typo --- .../lib/Target/DirectX/DXILPostOptimizationValidation.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index 762a67a4c7f76..bb1d9fa68528e 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -169,7 +169,7 @@ reportDescriptorTableMixingTypes(Module &M, uint32_t Location, } static void -reportOverlowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { +reportOverflowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { SmallString<128> Message; raw_svector_ostream OS(Message); OS << "Cannot append range with implicit lower " @@ -288,19 +288,19 @@ static void validateDescriptorTables(Module &M, Offset = Range.OffsetInDescriptorsFromTableStart; if (Offset > ~0U) - reportOverlowingRange(M, Range); + reportOverflowingRange(M, Range); if (Range.NumDescriptors == ~0U) { AppendingOffset = (uint64_t)~0U + (uint64_t)1ULL; } else { uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + (uint64_t)Range.NumDescriptors - (uint64_t)1U; if (UpperBound > ~0U) - reportOverlowingRange(M, Range); + reportOverflowingRange(M, Range); uint64_t AppendingUpperBound = (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; if (AppendingUpperBound > ~0U) - reportOverlowingRange(M, Range); + reportOverflowingRange(M, Range); AppendingOffset = Offset + Range.NumDescriptors; } From 81261ff42897288b8d4cd625511257f6d113ed57 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 13:45:17 -0700 Subject: [PATCH 07/18] clean --- llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index bb1d9fa68528e..c9b558a5c62bc 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -169,7 +169,8 @@ reportDescriptorTableMixingTypes(Module &M, uint32_t Location, } static void -reportOverflowingRange(Module &M, const dxbc::RTS0::v2::DescriptorRange &Range) { +reportOverflowingRange(Module &M, + const dxbc::RTS0::v2::DescriptorRange &Range) { SmallString<128> Message; raw_svector_ostream OS(Message); OS << "Cannot append range with implicit lower " From e5e73febb73594b3e51b4deaabf9d913b987f2b5 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 16:58:24 -0700 Subject: [PATCH 08/18] address comments --- .../Frontend/HLSL/RootSignatureMetadata.h | 80 +++++++++++ .../Frontend/HLSL/RootSignatureMetadata.cpp | 91 ++++++++++++ .../DXILPostOptimizationValidation.cpp | 131 +----------------- 3 files changed, 178 insertions(+), 124 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index 0bd0774641287..87b950d77df53 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -15,9 +15,11 @@ #define LLVM_FRONTEND_HLSL_ROOTSIGNATUREMETADATA_H #include "llvm/ADT/StringRef.h" +#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include "llvm/IR/Constants.h" #include "llvm/MC/DXContainerRootSignature.h" +#include namespace llvm { class LLVMContext; @@ -27,6 +29,38 @@ class Metadata; namespace hlsl { namespace rootsig { +inline dxil::ResourceClass +toResourceClass(dxbc::DescriptorRangeType RangeType) { + using namespace dxbc; + switch (RangeType) { + case DescriptorRangeType::SRV: + return dxil::ResourceClass::SRV; + case DescriptorRangeType::UAV: + return dxil::ResourceClass::UAV; + case DescriptorRangeType::CBV: + return dxil::ResourceClass::CBuffer; + case DescriptorRangeType::Sampler: + return dxil::ResourceClass::Sampler; + } +} + +inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { + using namespace dxbc; + switch (Type) { + case RootParameterType::Constants32Bit: + return dxil::ResourceClass::CBuffer; + case RootParameterType::SRV: + return dxil::ResourceClass::SRV; + case RootParameterType::UAV: + return dxil::ResourceClass::UAV; + case RootParameterType::CBV: + return dxil::ResourceClass::CBuffer; + case dxbc::RootParameterType::DescriptorTable: + break; + } + llvm_unreachable("Unconvertible RootParameterType"); +} + template class RootSignatureValidationError : public ErrorInfo> { @@ -47,6 +81,52 @@ class RootSignatureValidationError } }; +class TableRegisterOverflowError + : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Register; + uint32_t Space; + + TableRegisterOverflowError(dxbc::DescriptorRangeType Type, uint32_t Register, + uint32_t Space) + : Type(Type), Register(Register), Space(Space) {} + + void log(raw_ostream &OS) const override { + OS << "Cannot append range with implicit lower " + << "bound after an unbounded range " + << getResourceClassName(toResourceClass(Type)) + << "(register=" << Register << ", space=" << Space + << ") exceeds maximum allowed value."; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + +class TableSamplerMixinError : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Location; + + TableSamplerMixinError(dxbc::DescriptorRangeType Type, uint32_t Location) + : Type(Type), Location(Location) {} + + void log(raw_ostream &OS) const override { + OS << "Samplers cannot be mixed with other " + << "resource types in a descriptor table, " + << getResourceClassName(toResourceClass(Type)) + << "(location=" << Location << ")"; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + class GenericRSMetadataError : public ErrorInfo { public: static char ID; diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 6d89fa7b1222c..3878026518882 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -15,7 +15,9 @@ #include "llvm/Frontend/HLSL/RootSignatureValidations.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Metadata.h" +#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" +#include using namespace llvm; @@ -26,6 +28,8 @@ namespace rootsig { char GenericRSMetadataError::ID; char InvalidRSMetadataFormat::ID; char InvalidRSMetadataValue::ID; +char TableSamplerMixinError::ID; +char TableRegisterOverflowError::ID; template char RootSignatureValidationError::ID; static std::optional extractMdIntValue(MDNode *Node, @@ -525,6 +529,83 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD, llvm_unreachable("Unhandled RootSignatureElementKind enum."); } +Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table, + uint32_t Location) { + bool HasSampler = false; + bool HasOtherRangeType = false; + dxbc::DescriptorRangeType OtherRangeType; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + dxbc::DescriptorRangeType RangeType = + static_cast(Range.RangeType); + + if (RangeType == dxbc::DescriptorRangeType::Sampler) { + HasSampler = true; + } else { + HasOtherRangeType = true; + OtherRangeType = RangeType; + } + } + + // Samplers cannot be mixed with other resources in a descriptor table. + if (HasSampler && HasOtherRangeType) + return make_error(OtherRangeType, Location); + return Error::success(); +} + +/** This validation logic was extracted from the DXC codebase + * https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205 + * + * It checks if the registers in a descriptor table are overflowing, meaning, + * they are trying to bind a register larger than MAX_UINT. + * This will usually happen when the descriptor table defined a range after an + * unbounded range, which would lead to an overflow in the register; + * Or if trying append a bunch or really large ranges. + **/ +Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, + uint32_t Location) { + uint64_t AppendingRegister = 0; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + + dxbc::DescriptorRangeType RangeType = + static_cast(Range.RangeType); + + uint64_t Register = AppendingRegister; + + // Checks if the current register should be appended to the previous range. + if (Range.OffsetInDescriptorsFromTableStart != ~0U) + Register = Range.OffsetInDescriptorsFromTableStart; + + // Check for overflow in the register value. + if (Register > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + // Is the current range unbounded? + if (Range.NumDescriptors == ~0U) { + // No ranges should be appended to an unbounded range. + AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL; + } else { + // Is the defined range, overflowing? + uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (UpperBound > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + + // If we append this range, will it overflow? + uint64_t AppendingUpperBound = + (uint64_t)Register + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (AppendingUpperBound > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + AppendingRegister = Register + Range.NumDescriptors; + } + } + + return Error::success(); +} + Error MetadataParser::validateRootSignature( const mcdxbc::RootSignatureDesc &RSD) { Error DeferredErrs = Error::success(); @@ -609,6 +690,16 @@ Error MetadataParser::validateRootSignature( joinErrors(std::move(DeferredErrs), make_error>( "DescriptorFlag", Range.Flags)); + + if (Error Err = + validateDescriptorTableSamplerMixin(Table, Info.Location)) { + DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err)); + } + + if (Error Err = + validateDescriptorTableRegisterOverflow(Table, Info.Location)) { + DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err)); + } } break; } diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index c9b558a5c62bc..492c0436d0178 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -14,6 +14,7 @@ #include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/Analysis/DXILResource.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Frontend/HLSL/RootSignatureMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicsDirectX.h" @@ -27,37 +28,6 @@ using namespace llvm; using namespace llvm::dxil; -static ResourceClass toResourceClass(dxbc::DescriptorRangeType RangeType) { - using namespace dxbc; - switch (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; - } -} - -static ResourceClass toResourceClass(dxbc::RootParameterType Type) { - using namespace dxbc; - switch (Type) { - case RootParameterType::Constants32Bit: - return ResourceClass::CBuffer; - case RootParameterType::SRV: - return ResourceClass::SRV; - case RootParameterType::UAV: - return ResourceClass::UAV; - case RootParameterType::CBV: - return ResourceClass::CBuffer; - case dxbc::RootParameterType::DescriptorTable: - break; - } - llvm_unreachable("Unconvertible RootParameterType"); -} - static void reportInvalidDirection(Module &M, DXILResourceMap &DRM) { for (const auto &UAV : DRM.uavs()) { if (UAV.CounterDirection != ResourceCounterDirection::Invalid) @@ -155,33 +125,6 @@ reportRegNotBound(Module &M, ResourceClass Class, M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } -static void -reportDescriptorTableMixingTypes(Module &M, uint32_t Location, - dxbc::DescriptorRangeType RangeType) { - SmallString<128> Message; - raw_svector_ostream OS(Message); - OS << "Samplers cannot be mixed with other " - << "resource types in a descriptor table, " - << getResourceClassName(toResourceClass(RangeType)) - << "(location=" << Location << ")"; - - M.getContext().diagnose(DiagnosticInfoGeneric(Message)); -} - -static void -reportOverflowingRange(Module &M, - const dxbc::RTS0::v2::DescriptorRange &Range) { - SmallString<128> Message; - raw_svector_ostream OS(Message); - OS << "Cannot append range with implicit lower " - << "bound after an unbounded range " - << getResourceClassName(toResourceClass( - static_cast(Range.RangeType))) - << "(register=" << Range.BaseShaderRegister - << ", space=" << Range.RegisterSpace << ") exceeds maximum allowed value."; - M.getContext().diagnose(DiagnosticInfoGeneric(Message)); -} - static void reportInvalidHandleTy( Module &M, const llvm::ArrayRef &RDs, const iterator_range::iterator> @@ -263,66 +206,6 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, return RDs; } -static void validateDescriptorTables(Module &M, - const mcdxbc::RootSignatureDesc &RSD) { - for (const mcdxbc::RootParameterInfo &ParamInfo : RSD.ParametersContainer) { - if (static_cast(ParamInfo.Header.ParameterType) != - dxbc::RootParameterType::DescriptorTable) - continue; - - mcdxbc::DescriptorTable Table = - RSD.ParametersContainer.getDescriptorTable(ParamInfo.Location); - - bool HasSampler = false; - bool HasOtherRangeType = false; - dxbc::DescriptorRangeType OtherRangeType; - uint32_t OtherRangeTypeLocation = 0; - - uint64_t AppendingOffset = 0; - - for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { - dxbc::DescriptorRangeType RangeType = - static_cast(Range.RangeType); - - uint64_t Offset = AppendingOffset; - if (Range.OffsetInDescriptorsFromTableStart != ~0U) - Offset = Range.OffsetInDescriptorsFromTableStart; - - if (Offset > ~0U) - reportOverflowingRange(M, Range); - if (Range.NumDescriptors == ~0U) { - AppendingOffset = (uint64_t)~0U + (uint64_t)1ULL; - } else { - uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + - (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (UpperBound > ~0U) - reportOverflowingRange(M, Range); - - uint64_t AppendingUpperBound = - (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (AppendingUpperBound > ~0U) - reportOverflowingRange(M, Range); - AppendingOffset = Offset + Range.NumDescriptors; - } - - if (RangeType == dxbc::DescriptorRangeType::Sampler) { - HasSampler = true; - } else { - HasOtherRangeType = true; - OtherRangeType = RangeType; - OtherRangeTypeLocation = ParamInfo.Location; - } - } - - // Samplers cannot be mixed with other resources in a descriptor table. - if (HasSampler && HasOtherRangeType) { - reportDescriptorTableMixingTypes(M, OtherRangeTypeLocation, - OtherRangeType); - continue; - } - } -} - static void validateRootSignatureBindings(Module &M, const mcdxbc::RootSignatureDesc &RSD, dxil::ModuleMetadataInfo &MMI, @@ -354,10 +237,11 @@ static void validateRootSignatureBindings(Module &M, case dxbc::RootParameterType::CBV: { dxbc::RTS0::v2::RootDescriptor Desc = RSD.ParametersContainer.getRootDescriptor(ParamInfo.Location); - Builder.trackBinding(toResourceClass(static_cast( - ParamInfo.Header.ParameterType)), - Desc.RegisterSpace, Desc.ShaderRegister, - Desc.ShaderRegister, &IDs.emplace_back()); + Builder.trackBinding( + hlsl::rootsig::toResourceClass(static_cast( + ParamInfo.Header.ParameterType)), + Desc.RegisterSpace, Desc.ShaderRegister, Desc.ShaderRegister, + &IDs.emplace_back()); break; } @@ -371,7 +255,7 @@ static void validateRootSignatureBindings(Module &M, ? Range.BaseShaderRegister : Range.BaseShaderRegister + Range.NumDescriptors - 1; Builder.trackBinding( - toResourceClass( + hlsl::rootsig::toResourceClass( static_cast(Range.RangeType)), Range.RegisterSpace, Range.BaseShaderRegister, UpperBound, &IDs.emplace_back()); @@ -439,7 +323,6 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI)) { validateRootSignatureBindings(M, *RSD, MMI, DRM); - validateDescriptorTables(M, *RSD); } } From ad0e1f3f2539316571d91ca4451b0b467afe631e Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 16:58:24 -0700 Subject: [PATCH 09/18] address comments --- .../Frontend/HLSL/RootSignatureMetadata.h | 79 +++++++++++ .../Frontend/HLSL/RootSignatureMetadata.cpp | 91 ++++++++++++ .../DXILPostOptimizationValidation.cpp | 131 +----------------- 3 files changed, 177 insertions(+), 124 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index 0bd0774641287..308d04a5ee1e9 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -15,9 +15,11 @@ #define LLVM_FRONTEND_HLSL_ROOTSIGNATUREMETADATA_H #include "llvm/ADT/StringRef.h" +#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include "llvm/IR/Constants.h" #include "llvm/MC/DXContainerRootSignature.h" +#include namespace llvm { class LLVMContext; @@ -27,6 +29,38 @@ class Metadata; namespace hlsl { namespace rootsig { +inline dxil::ResourceClass +toResourceClass(dxbc::DescriptorRangeType RangeType) { + using namespace dxbc; + switch (RangeType) { + case DescriptorRangeType::SRV: + return dxil::ResourceClass::SRV; + case DescriptorRangeType::UAV: + return dxil::ResourceClass::UAV; + case DescriptorRangeType::CBV: + return dxil::ResourceClass::CBuffer; + case DescriptorRangeType::Sampler: + return dxil::ResourceClass::Sampler; + } +} + +inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { + using namespace dxbc; + switch (Type) { + case RootParameterType::Constants32Bit: + return dxil::ResourceClass::CBuffer; + case RootParameterType::SRV: + return dxil::ResourceClass::SRV; + case RootParameterType::UAV: + return dxil::ResourceClass::UAV; + case RootParameterType::CBV: + return dxil::ResourceClass::CBuffer; + case dxbc::RootParameterType::DescriptorTable: + break; + } + llvm_unreachable("Unconvertible RootParameterType"); +} + template class RootSignatureValidationError : public ErrorInfo> { @@ -47,6 +81,51 @@ class RootSignatureValidationError } }; +class TableRegisterOverflowError + : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Register; + uint32_t Space; + + TableRegisterOverflowError(dxbc::DescriptorRangeType Type, uint32_t Register, + uint32_t Space) + : Type(Type), Register(Register), Space(Space) {} + + void log(raw_ostream &OS) const override { + OS << "Cannot bind resource of type " + << getResourceClassName(toResourceClass(Type)) + << "(register=" << Register << ", space=" << Space + << "), it exceeds the maximum allowed register value."; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + +class TableSamplerMixinError : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Location; + + TableSamplerMixinError(dxbc::DescriptorRangeType Type, uint32_t Location) + : Type(Type), Location(Location) {} + + void log(raw_ostream &OS) const override { + OS << "Samplers cannot be mixed with other " + << "resource types in a descriptor table, " + << getResourceClassName(toResourceClass(Type)) + << "(location=" << Location << ")"; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + class GenericRSMetadataError : public ErrorInfo { public: static char ID; diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 6d89fa7b1222c..3878026518882 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -15,7 +15,9 @@ #include "llvm/Frontend/HLSL/RootSignatureValidations.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Metadata.h" +#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" +#include using namespace llvm; @@ -26,6 +28,8 @@ namespace rootsig { char GenericRSMetadataError::ID; char InvalidRSMetadataFormat::ID; char InvalidRSMetadataValue::ID; +char TableSamplerMixinError::ID; +char TableRegisterOverflowError::ID; template char RootSignatureValidationError::ID; static std::optional extractMdIntValue(MDNode *Node, @@ -525,6 +529,83 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD, llvm_unreachable("Unhandled RootSignatureElementKind enum."); } +Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table, + uint32_t Location) { + bool HasSampler = false; + bool HasOtherRangeType = false; + dxbc::DescriptorRangeType OtherRangeType; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + dxbc::DescriptorRangeType RangeType = + static_cast(Range.RangeType); + + if (RangeType == dxbc::DescriptorRangeType::Sampler) { + HasSampler = true; + } else { + HasOtherRangeType = true; + OtherRangeType = RangeType; + } + } + + // Samplers cannot be mixed with other resources in a descriptor table. + if (HasSampler && HasOtherRangeType) + return make_error(OtherRangeType, Location); + return Error::success(); +} + +/** This validation logic was extracted from the DXC codebase + * https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205 + * + * It checks if the registers in a descriptor table are overflowing, meaning, + * they are trying to bind a register larger than MAX_UINT. + * This will usually happen when the descriptor table defined a range after an + * unbounded range, which would lead to an overflow in the register; + * Or if trying append a bunch or really large ranges. + **/ +Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, + uint32_t Location) { + uint64_t AppendingRegister = 0; + + for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { + + dxbc::DescriptorRangeType RangeType = + static_cast(Range.RangeType); + + uint64_t Register = AppendingRegister; + + // Checks if the current register should be appended to the previous range. + if (Range.OffsetInDescriptorsFromTableStart != ~0U) + Register = Range.OffsetInDescriptorsFromTableStart; + + // Check for overflow in the register value. + if (Register > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + // Is the current range unbounded? + if (Range.NumDescriptors == ~0U) { + // No ranges should be appended to an unbounded range. + AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL; + } else { + // Is the defined range, overflowing? + uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (UpperBound > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + + // If we append this range, will it overflow? + uint64_t AppendingUpperBound = + (uint64_t)Register + (uint64_t)Range.NumDescriptors - (uint64_t)1U; + if (AppendingUpperBound > ~0U) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + AppendingRegister = Register + Range.NumDescriptors; + } + } + + return Error::success(); +} + Error MetadataParser::validateRootSignature( const mcdxbc::RootSignatureDesc &RSD) { Error DeferredErrs = Error::success(); @@ -609,6 +690,16 @@ Error MetadataParser::validateRootSignature( joinErrors(std::move(DeferredErrs), make_error>( "DescriptorFlag", Range.Flags)); + + if (Error Err = + validateDescriptorTableSamplerMixin(Table, Info.Location)) { + DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err)); + } + + if (Error Err = + validateDescriptorTableRegisterOverflow(Table, Info.Location)) { + DeferredErrs = joinErrors(std::move(DeferredErrs), std::move(Err)); + } } break; } diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index c9b558a5c62bc..492c0436d0178 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -14,6 +14,7 @@ #include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/Analysis/DXILResource.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Frontend/HLSL/RootSignatureMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicsDirectX.h" @@ -27,37 +28,6 @@ using namespace llvm; using namespace llvm::dxil; -static ResourceClass toResourceClass(dxbc::DescriptorRangeType RangeType) { - using namespace dxbc; - switch (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; - } -} - -static ResourceClass toResourceClass(dxbc::RootParameterType Type) { - using namespace dxbc; - switch (Type) { - case RootParameterType::Constants32Bit: - return ResourceClass::CBuffer; - case RootParameterType::SRV: - return ResourceClass::SRV; - case RootParameterType::UAV: - return ResourceClass::UAV; - case RootParameterType::CBV: - return ResourceClass::CBuffer; - case dxbc::RootParameterType::DescriptorTable: - break; - } - llvm_unreachable("Unconvertible RootParameterType"); -} - static void reportInvalidDirection(Module &M, DXILResourceMap &DRM) { for (const auto &UAV : DRM.uavs()) { if (UAV.CounterDirection != ResourceCounterDirection::Invalid) @@ -155,33 +125,6 @@ reportRegNotBound(Module &M, ResourceClass Class, M.getContext().diagnose(DiagnosticInfoGeneric(Message)); } -static void -reportDescriptorTableMixingTypes(Module &M, uint32_t Location, - dxbc::DescriptorRangeType RangeType) { - SmallString<128> Message; - raw_svector_ostream OS(Message); - OS << "Samplers cannot be mixed with other " - << "resource types in a descriptor table, " - << getResourceClassName(toResourceClass(RangeType)) - << "(location=" << Location << ")"; - - M.getContext().diagnose(DiagnosticInfoGeneric(Message)); -} - -static void -reportOverflowingRange(Module &M, - const dxbc::RTS0::v2::DescriptorRange &Range) { - SmallString<128> Message; - raw_svector_ostream OS(Message); - OS << "Cannot append range with implicit lower " - << "bound after an unbounded range " - << getResourceClassName(toResourceClass( - static_cast(Range.RangeType))) - << "(register=" << Range.BaseShaderRegister - << ", space=" << Range.RegisterSpace << ") exceeds maximum allowed value."; - M.getContext().diagnose(DiagnosticInfoGeneric(Message)); -} - static void reportInvalidHandleTy( Module &M, const llvm::ArrayRef &RDs, const iterator_range::iterator> @@ -263,66 +206,6 @@ getRootDescriptorsBindingInfo(const mcdxbc::RootSignatureDesc &RSD, return RDs; } -static void validateDescriptorTables(Module &M, - const mcdxbc::RootSignatureDesc &RSD) { - for (const mcdxbc::RootParameterInfo &ParamInfo : RSD.ParametersContainer) { - if (static_cast(ParamInfo.Header.ParameterType) != - dxbc::RootParameterType::DescriptorTable) - continue; - - mcdxbc::DescriptorTable Table = - RSD.ParametersContainer.getDescriptorTable(ParamInfo.Location); - - bool HasSampler = false; - bool HasOtherRangeType = false; - dxbc::DescriptorRangeType OtherRangeType; - uint32_t OtherRangeTypeLocation = 0; - - uint64_t AppendingOffset = 0; - - for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { - dxbc::DescriptorRangeType RangeType = - static_cast(Range.RangeType); - - uint64_t Offset = AppendingOffset; - if (Range.OffsetInDescriptorsFromTableStart != ~0U) - Offset = Range.OffsetInDescriptorsFromTableStart; - - if (Offset > ~0U) - reportOverflowingRange(M, Range); - if (Range.NumDescriptors == ~0U) { - AppendingOffset = (uint64_t)~0U + (uint64_t)1ULL; - } else { - uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + - (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (UpperBound > ~0U) - reportOverflowingRange(M, Range); - - uint64_t AppendingUpperBound = - (uint64_t)Offset + (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (AppendingUpperBound > ~0U) - reportOverflowingRange(M, Range); - AppendingOffset = Offset + Range.NumDescriptors; - } - - if (RangeType == dxbc::DescriptorRangeType::Sampler) { - HasSampler = true; - } else { - HasOtherRangeType = true; - OtherRangeType = RangeType; - OtherRangeTypeLocation = ParamInfo.Location; - } - } - - // Samplers cannot be mixed with other resources in a descriptor table. - if (HasSampler && HasOtherRangeType) { - reportDescriptorTableMixingTypes(M, OtherRangeTypeLocation, - OtherRangeType); - continue; - } - } -} - static void validateRootSignatureBindings(Module &M, const mcdxbc::RootSignatureDesc &RSD, dxil::ModuleMetadataInfo &MMI, @@ -354,10 +237,11 @@ static void validateRootSignatureBindings(Module &M, case dxbc::RootParameterType::CBV: { dxbc::RTS0::v2::RootDescriptor Desc = RSD.ParametersContainer.getRootDescriptor(ParamInfo.Location); - Builder.trackBinding(toResourceClass(static_cast( - ParamInfo.Header.ParameterType)), - Desc.RegisterSpace, Desc.ShaderRegister, - Desc.ShaderRegister, &IDs.emplace_back()); + Builder.trackBinding( + hlsl::rootsig::toResourceClass(static_cast( + ParamInfo.Header.ParameterType)), + Desc.RegisterSpace, Desc.ShaderRegister, Desc.ShaderRegister, + &IDs.emplace_back()); break; } @@ -371,7 +255,7 @@ static void validateRootSignatureBindings(Module &M, ? Range.BaseShaderRegister : Range.BaseShaderRegister + Range.NumDescriptors - 1; Builder.trackBinding( - toResourceClass( + hlsl::rootsig::toResourceClass( static_cast(Range.RangeType)), Range.RegisterSpace, Range.BaseShaderRegister, UpperBound, &IDs.emplace_back()); @@ -439,7 +323,6 @@ static void reportErrors(Module &M, DXILResourceMap &DRM, if (mcdxbc::RootSignatureDesc *RSD = getRootSignature(RSBI, MMI)) { validateRootSignatureBindings(M, *RSD, MMI, DRM); - validateDescriptorTables(M, *RSD); } } From 8a4ee3c7077e216aadc94749a8ba2adf9e05a822 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Tue, 12 Aug 2025 17:12:04 -0700 Subject: [PATCH 10/18] fix comment --- llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 3878026518882..dd183b6446a48 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -558,9 +558,8 @@ Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table, * * It checks if the registers in a descriptor table are overflowing, meaning, * they are trying to bind a register larger than MAX_UINT. - * This will usually happen when the descriptor table defined a range after an - * unbounded range, which would lead to an overflow in the register; - * Or if trying append a bunch or really large ranges. + * This will usually happen when the descriptor table appends a resource + * after an unbounded range. **/ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, uint32_t Location) { From 23537b5fb06ea9eeef2fee0db4fa135ed301146e Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Thu, 14 Aug 2025 13:31:49 -0700 Subject: [PATCH 11/18] addressing comment --- llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h | 6 +++--- .../rootsignature-validation-fail-appending-overflow.ll | 4 +--- .../rootsignature-validation-fail-register-overflow.ll | 3 +-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index 036b07f0d4b58..2a9644fff1923 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -93,10 +93,10 @@ class TableRegisterOverflowError : Type(Type), Register(Register), Space(Space) {} void log(raw_ostream &OS) const override { - OS << "Cannot bind resource of type " + OS << "Cannot append range with implicit lower bound after an unbounded " + "range " << getResourceClassName(toResourceClass(Type)) - << "(register=" << Register << ", space=" << Space - << "), it exceeds the maximum allowed register value."; + << "(register=" << Register << ", space=" << Space << ")."; } std::error_code convertToErrorCode() const override { diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll index 93d6cb4c17f50..2b5ce4f52b94e 100644 --- a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-overflow.ll @@ -1,7 +1,7 @@ ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s ; This test check if a resource is implicitly overflowing. That means, it is appending a resource after an unbounded range. -; CHECK: error: Cannot bind resource of type UAV(register=0, space=0), it exceeds the maximum allowed register value. +; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=0, space=0). @TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 @@ -17,5 +17,3 @@ entry: !3 = !{!"DescriptorTable", i32 0, !4, !5} !4 = !{!"UAV", i32 -1, i32 1, i32 0, i32 2, i32 0} !5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0} - - diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll index 632d5ca7978f3..65542a6ca98f2 100644 --- a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll @@ -1,5 +1,5 @@ ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s -; CHECK: error: Cannot bind resource of type UAV(register=4294967295, space=0), it exceeds the maximum allowed register value. +; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=4294967295, space=0). @TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 define void @CSMain() "hlsl.shader"="compute" { @@ -13,4 +13,3 @@ entry: !1 = !{!3} !3 = !{!"DescriptorTable", i32 0, !4} !4 = !{!"UAV", i32 100, i32 4294967295, i32 0, i32 -1, i32 0} - From d3349cec0490c0b8d26d5ff21f32b2b292bf2f28 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Thu, 14 Aug 2025 17:46:40 -0700 Subject: [PATCH 12/18] clean up --- llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h | 6 +++--- llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index c817782f21e58..a112fe6ade7c6 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -15,7 +15,6 @@ #define LLVM_FRONTEND_HLSL_ROOTSIGNATUREMETADATA_H #include "llvm/ADT/StringRef.h" -#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include "llvm/IR/Constants.h" #include "llvm/MC/DXContainerRootSignature.h" @@ -42,6 +41,7 @@ toResourceClass(dxbc::DescriptorRangeType RangeType) { case DescriptorRangeType::Sampler: return dxil::ResourceClass::Sampler; } + llvm_unreachable("Unknown DescriptorRangeType"); } inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { @@ -56,9 +56,9 @@ inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { case RootParameterType::CBV: return dxil::ResourceClass::CBuffer; case dxbc::RootParameterType::DescriptorTable: - break; + llvm_unreachable("DescriptorTable is not convertible to ResourceClass"); } - llvm_unreachable("Unconvertible RootParameterType"); + llvm_unreachable("Unknown RootParameterType"); } template diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 47957f08fc008..c2d32ad25fca0 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -15,9 +15,7 @@ #include "llvm/Frontend/HLSL/RootSignatureValidations.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Metadata.h" -#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" -#include using namespace llvm; From bf5714d7abf8d3dfe4d1e8d108b95ac113dcf092 Mon Sep 17 00:00:00 2001 From: Joao Saffran <{ID}+{username}@users.noreply.github.com> Date: Thu, 14 Aug 2025 17:49:37 -0700 Subject: [PATCH 13/18] moving code to where I think make sense --- llvm/include/llvm/BinaryFormat/DXContainer.h | 33 +++++++++++++++++++ .../Frontend/HLSL/RootSignatureMetadata.h | 33 ------------------- .../DXILPostOptimizationValidation.cpp | 12 +++---- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index f74c9775cb3f3..355a86420d29d 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -214,6 +214,39 @@ enum class ShaderVisibility : uint32_t { #include "DXContainerConstants.def" }; +inline dxil::ResourceClass +toResourceClass(dxbc::DescriptorRangeType RangeType) { + using namespace dxbc; + switch (RangeType) { + case DescriptorRangeType::SRV: + return dxil::ResourceClass::SRV; + case DescriptorRangeType::UAV: + return dxil::ResourceClass::UAV; + case DescriptorRangeType::CBV: + return dxil::ResourceClass::CBuffer; + case DescriptorRangeType::Sampler: + return dxil::ResourceClass::Sampler; + } + llvm_unreachable("Unknown DescriptorRangeType"); +} + +inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { + using namespace dxbc; + switch (Type) { + case RootParameterType::Constants32Bit: + return dxil::ResourceClass::CBuffer; + case RootParameterType::SRV: + return dxil::ResourceClass::SRV; + case RootParameterType::UAV: + return dxil::ResourceClass::UAV; + case RootParameterType::CBV: + return dxil::ResourceClass::CBuffer; + case dxbc::RootParameterType::DescriptorTable: + llvm_unreachable("DescriptorTable is not convertible to ResourceClass"); + } + llvm_unreachable("Unknown RootParameterType"); +} + LLVM_ABI ArrayRef> getShaderVisibility(); #define SHADER_VISIBILITY(Val, Enum) \ diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index a112fe6ade7c6..096cd9dded9eb 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -28,39 +28,6 @@ class Metadata; namespace hlsl { namespace rootsig { -inline dxil::ResourceClass -toResourceClass(dxbc::DescriptorRangeType RangeType) { - using namespace dxbc; - switch (RangeType) { - case DescriptorRangeType::SRV: - return dxil::ResourceClass::SRV; - case DescriptorRangeType::UAV: - return dxil::ResourceClass::UAV; - case DescriptorRangeType::CBV: - return dxil::ResourceClass::CBuffer; - case DescriptorRangeType::Sampler: - return dxil::ResourceClass::Sampler; - } - llvm_unreachable("Unknown DescriptorRangeType"); -} - -inline dxil::ResourceClass toResourceClass(dxbc::RootParameterType Type) { - using namespace dxbc; - switch (Type) { - case RootParameterType::Constants32Bit: - return dxil::ResourceClass::CBuffer; - case RootParameterType::SRV: - return dxil::ResourceClass::SRV; - case RootParameterType::UAV: - return dxil::ResourceClass::UAV; - case RootParameterType::CBV: - return dxil::ResourceClass::CBuffer; - case dxbc::RootParameterType::DescriptorTable: - llvm_unreachable("DescriptorTable is not convertible to ResourceClass"); - } - llvm_unreachable("Unknown RootParameterType"); -} - template class RootSignatureValidationError : public ErrorInfo> { diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp index b78cc63388179..1d2c9a35c1184 100644 --- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp +++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp @@ -13,7 +13,6 @@ #include "llvm/ADT/SmallString.h" #include "llvm/Analysis/DXILMetadataAnalysis.h" #include "llvm/Analysis/DXILResource.h" -#include "llvm/Frontend/HLSL/RootSignatureMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicsDirectX.h" @@ -237,10 +236,11 @@ static void validateRootSignatureBindings(Module &M, case dxbc::RootParameterType::CBV: { dxbc::RTS0::v2::RootDescriptor Desc = RSD.ParametersContainer.getRootDescriptor(ParamInfo.Location); - Builder.trackBinding(hlsl::rootsig::toResourceClass(static_cast( - ParamInfo.Header.ParameterType)), - Desc.RegisterSpace, Desc.ShaderRegister, - Desc.ShaderRegister, &ParamInfo); + Builder.trackBinding( + dxbc::toResourceClass(static_cast( + ParamInfo.Header.ParameterType)), + Desc.RegisterSpace, Desc.ShaderRegister, Desc.ShaderRegister, + &ParamInfo); break; } @@ -254,7 +254,7 @@ static void validateRootSignatureBindings(Module &M, ? Range.BaseShaderRegister : Range.BaseShaderRegister + Range.NumDescriptors - 1; Builder.trackBinding( - hlsl::rootsig::toResourceClass( + dxbc::toResourceClass( static_cast(Range.RangeType)), Range.RegisterSpace, Range.BaseShaderRegister, UpperBound, &ParamInfo); From 34619da2ba69efa09d1a5d0efa7fb98ab6505953 Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Fri, 22 Aug 2025 15:20:46 -0700 Subject: [PATCH 14/18] moving offset logic into shared validations --- .../Frontend/HLSL/RootSignatureValidations.h | 3 ++ .../Frontend/HLSL/RootSignatureMetadata.cpp | 42 ++----------------- .../HLSL/RootSignatureValidations.cpp | 42 +++++++++++++++++++ 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index fde32a1fff591..c6a2886e89118 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -40,6 +40,9 @@ LLVM_ABI bool verifyMaxAnisotropy(uint32_t MaxAnisotropy); LLVM_ABI bool verifyComparisonFunc(uint32_t ComparisonFunc); LLVM_ABI bool verifyBorderColor(uint32_t BorderColor); LLVM_ABI bool verifyLOD(float LOD); +LLVM_ABI bool verifyOffsetOverflowing( + uint64_t &AppendingRegister, uint32_t OffsetInDescriptorsFromTableStart, + uint32_t BaseRegister, uint32_t Space, uint32_t NumDescriptors); } // namespace rootsig } // namespace hlsl diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index c2d32ad25fca0..40b1f10e09ae9 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -539,53 +539,19 @@ Error validateDescriptorTableSamplerMixin(mcdxbc::DescriptorTable Table, return Error::success(); } -/** This validation logic was extracted from the DXC codebase - * https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205 - * - * It checks if the registers in a descriptor table are overflowing, meaning, - * they are trying to bind a register larger than MAX_UINT. - * This will usually happen when the descriptor table appends a resource - * after an unbounded range. - **/ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, uint32_t Location) { uint64_t AppendingRegister = 0; for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { - dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); - - uint64_t Register = AppendingRegister; - - // Checks if the current register should be appended to the previous range. - if (Range.OffsetInDescriptorsFromTableStart != ~0U) - Register = Range.OffsetInDescriptorsFromTableStart; - - // Check for overflow in the register value. - if (Register > ~0U) + if (verifyOffsetOverflowing(AppendingRegister, + Range.OffsetInDescriptorsFromTableStart, + Range.BaseShaderRegister, Range.RegisterSpace, + Range.NumDescriptors)) return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); - // Is the current range unbounded? - if (Range.NumDescriptors == ~0U) { - // No ranges should be appended to an unbounded range. - AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL; - } else { - // Is the defined range, overflowing? - uint64_t UpperBound = (uint64_t)Range.BaseShaderRegister + - (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (UpperBound > ~0U) - return make_error( - RangeType, Range.BaseShaderRegister, Range.RegisterSpace); - - // If we append this range, will it overflow? - uint64_t AppendingUpperBound = - (uint64_t)Register + (uint64_t)Range.NumDescriptors - (uint64_t)1U; - if (AppendingUpperBound > ~0U) - return make_error( - RangeType, Range.BaseShaderRegister, Range.RegisterSpace); - AppendingRegister = Register + Range.NumDescriptors; - } } return Error::success(); diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index 72308a3de5fd4..52cada81a7c44 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -180,6 +180,48 @@ bool verifyBorderColor(uint32_t BorderColor) { bool verifyLOD(float LOD) { return !std::isnan(LOD); } +/** This validation logic was extracted from the DXC codebase + * https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205 + * + * It checks if the registers in a descriptor table are overflowing, meaning, + * they are trying to bind a register larger than MAX_UINT. + * This will usually happen when the descriptor table appends a resource + * after an unbounded range. + **/ +bool verifyOffsetOverflowing(uint64_t &AppendingRegister, + uint32_t OffsetInDescriptorsFromTableStart, + uint32_t BaseRegister, uint32_t Space, + uint32_t NumDescriptors) { + uint64_t Register = AppendingRegister; + + // Checks if the current register should be appended to the previous range. + if (OffsetInDescriptorsFromTableStart != ~0U) + Register = OffsetInDescriptorsFromTableStart; + + // Check for overflow in the register value. + if (Register > ~0U) + return true; + // Is the current range unbounded? + if (NumDescriptors == ~0U) { + // No ranges should be appended to an unbounded range. + AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL; + } else { + // Is the defined range, overflowing? + uint64_t UpperBound = + (uint64_t)BaseRegister + (uint64_t)NumDescriptors - (uint64_t)1U; + if (UpperBound > ~0U) + return true; + + // If we append this range, will it overflow? + uint64_t AppendingUpperBound = + (uint64_t)Register + (uint64_t)NumDescriptors - (uint64_t)1U; + if (AppendingUpperBound > ~0U) + return true; + AppendingRegister = Register + NumDescriptors; + } + return false; +} + } // namespace rootsig } // namespace hlsl } // namespace llvm From 84d457985374fa8f24bdeac2da5c8d65cb5e502e Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Mon, 25 Aug 2025 13:08:50 -0700 Subject: [PATCH 15/18] refactoring suggested by inbelic --- .../Frontend/HLSL/RootSignatureValidations.h | 9 ++- .../Frontend/HLSL/RootSignatureMetadata.cpp | 18 ++++-- .../HLSL/RootSignatureValidations.cpp | 59 +++++++------------ 3 files changed, 42 insertions(+), 44 deletions(-) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index c6a2886e89118..15769b910a55d 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -17,6 +17,7 @@ #include "llvm/ADT/IntervalMap.h" #include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include "llvm/Support/Compiler.h" +#include namespace llvm { namespace hlsl { @@ -40,9 +41,11 @@ LLVM_ABI bool verifyMaxAnisotropy(uint32_t MaxAnisotropy); LLVM_ABI bool verifyComparisonFunc(uint32_t ComparisonFunc); LLVM_ABI bool verifyBorderColor(uint32_t BorderColor); LLVM_ABI bool verifyLOD(float LOD); -LLVM_ABI bool verifyOffsetOverflowing( - uint64_t &AppendingRegister, uint32_t OffsetInDescriptorsFromTableStart, - uint32_t BaseRegister, uint32_t Space, uint32_t NumDescriptors); +LLVM_ABI bool verifyRegisterOverflow(uint64_t Register, + uint32_t NumDescriptors); +LLVM_ABI uint64_t updateAppendingRegister(uint64_t Register, + uint32_t NumDescriptors); +LLVM_ABI bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register); } // namespace rootsig } // namespace hlsl diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 40b1f10e09ae9..7a94ad42d6fbf 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -546,12 +546,22 @@ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) { dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); - if (verifyOffsetOverflowing(AppendingRegister, - Range.OffsetInDescriptorsFromTableStart, - Range.BaseShaderRegister, Range.RegisterSpace, - Range.NumDescriptors)) + + if (verifyOffsetOverflow(Range.OffsetInDescriptorsFromTableStart, + AppendingRegister)) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + + if (verifyRegisterOverflow(Range.BaseShaderRegister, Range.NumDescriptors)) return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + + if (verifyRegisterOverflow(AppendingRegister, Range.NumDescriptors)) + return make_error( + RangeType, Range.BaseShaderRegister, Range.RegisterSpace); + + AppendingRegister = + updateAppendingRegister(AppendingRegister, Range.NumDescriptors); } return Error::success(); diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index 52cada81a7c44..ff420de392b94 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -180,48 +180,33 @@ bool verifyBorderColor(uint32_t BorderColor) { bool verifyLOD(float LOD) { return !std::isnan(LOD); } -/** This validation logic was extracted from the DXC codebase - * https://github.com/microsoft/DirectXShaderCompiler/blob/7a1b1df9b50a8350a63756720e85196e0285e664/lib/DxilRootSignature/DxilRootSignatureValidator.cpp#L205 - * - * It checks if the registers in a descriptor table are overflowing, meaning, - * they are trying to bind a register larger than MAX_UINT. - * This will usually happen when the descriptor table appends a resource - * after an unbounded range. - **/ -bool verifyOffsetOverflowing(uint64_t &AppendingRegister, - uint32_t OffsetInDescriptorsFromTableStart, - uint32_t BaseRegister, uint32_t Space, - uint32_t NumDescriptors) { - uint64_t Register = AppendingRegister; - - // Checks if the current register should be appended to the previous range. - if (OffsetInDescriptorsFromTableStart != ~0U) - Register = OffsetInDescriptorsFromTableStart; - - // Check for overflow in the register value. +bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register) { + if (Offset != ~0U) + Register = Offset; + if (Register > ~0U) return true; - // Is the current range unbounded? - if (NumDescriptors == ~0U) { - // No ranges should be appended to an unbounded range. - AppendingRegister = (uint64_t)~0U + (uint64_t)1ULL; - } else { - // Is the defined range, overflowing? - uint64_t UpperBound = - (uint64_t)BaseRegister + (uint64_t)NumDescriptors - (uint64_t)1U; - if (UpperBound > ~0U) - return true; - - // If we append this range, will it overflow? - uint64_t AppendingUpperBound = - (uint64_t)Register + (uint64_t)NumDescriptors - (uint64_t)1U; - if (AppendingUpperBound > ~0U) - return true; - AppendingRegister = Register + NumDescriptors; - } return false; } +bool verifyRegisterOverflow(uint64_t Register, uint32_t NumDescriptors) { + if (NumDescriptors == ~0U) + return false; + + uint64_t UpperBound = + (uint64_t)Register + (uint64_t)NumDescriptors - (uint64_t)1U; + if (UpperBound > ~0U) + return true; + + return false; +} + +uint64_t updateAppendingRegister(uint64_t AppendingRegisterRegister, + uint32_t NumDescriptors) { + if (NumDescriptors == ~0U) + return (uint64_t)~0U + (uint64_t)1ULL; + return AppendingRegisterRegister + NumDescriptors; +} } // namespace rootsig } // namespace hlsl } // namespace llvm From d2b4aea9307e2124ca84ef72e9ebbafaf3cd7e7a Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Mon, 25 Aug 2025 19:41:57 -0700 Subject: [PATCH 16/18] adding multiple error messages --- .../Frontend/HLSL/RootSignatureMetadata.h | 53 +++++++++++++++++-- .../Frontend/HLSL/RootSignatureValidations.h | 7 +-- .../Frontend/HLSL/RootSignatureMetadata.cpp | 23 +++++--- .../HLSL/RootSignatureValidations.cpp | 12 ++--- ...gnature-validation-fail-offset-overflow.ll | 16 ++++++ ...ature-validation-fail-register-overflow.ll | 2 +- 6 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h index 096cd9dded9eb..6788b0f9773bc 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h @@ -48,16 +48,15 @@ class RootSignatureValidationError } }; -class TableRegisterOverflowError - : public ErrorInfo { +class OffsetOverflowError : public ErrorInfo { public: static char ID; dxbc::DescriptorRangeType Type; uint32_t Register; uint32_t Space; - TableRegisterOverflowError(dxbc::DescriptorRangeType Type, uint32_t Register, - uint32_t Space) + OffsetOverflowError(dxbc::DescriptorRangeType Type, uint32_t Register, + uint32_t Space) : Type(Type), Register(Register), Space(Space) {} void log(raw_ostream &OS) const override { @@ -72,6 +71,52 @@ class TableRegisterOverflowError } }; +class ShaderRegisterOverflowError + : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Register; + uint32_t Space; + + ShaderRegisterOverflowError(dxbc::DescriptorRangeType Type, uint32_t Register, + uint32_t Space) + : Type(Type), Register(Register), Space(Space) {} + + void log(raw_ostream &OS) const override { + OS << "Overflow for shader register range: " + << getResourceClassName(toResourceClass(Type)) + << "(register=" << Register << ", space=" << Space << ")."; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + +class DescriptorRangeOverflowError + : public ErrorInfo { +public: + static char ID; + dxbc::DescriptorRangeType Type; + uint32_t Register; + uint32_t Space; + + DescriptorRangeOverflowError(dxbc::DescriptorRangeType Type, + uint32_t Register, uint32_t Space) + : Type(Type), Register(Register), Space(Space) {} + + void log(raw_ostream &OS) const override { + OS << "Overflow for descriptor range: " + << getResourceClassName(toResourceClass(Type)) + << "(register=" << Register << ", space=" << Space << ")."; + } + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; + class TableSamplerMixinError : public ErrorInfo { public: static char ID; diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index 15769b910a55d..1ec3c01dfdd2e 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -43,9 +43,10 @@ LLVM_ABI bool verifyBorderColor(uint32_t BorderColor); LLVM_ABI bool verifyLOD(float LOD); LLVM_ABI bool verifyRegisterOverflow(uint64_t Register, uint32_t NumDescriptors); -LLVM_ABI uint64_t updateAppendingRegister(uint64_t Register, - uint32_t NumDescriptors); -LLVM_ABI bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register); +LLVM_ABI uint64_t updateAppendingRegister(uint64_t AppendingRegisterRegister, + uint64_t NumDescriptors, + uint64_t Offset); +LLVM_ABI bool verifyOffsetOverflow(uint64_t Register); } // namespace rootsig } // namespace hlsl diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index 7a94ad42d6fbf..aa7a4d0e1e579 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -27,7 +27,10 @@ char GenericRSMetadataError::ID; char InvalidRSMetadataFormat::ID; char InvalidRSMetadataValue::ID; char TableSamplerMixinError::ID; -char TableRegisterOverflowError::ID; +char ShaderRegisterOverflowError::ID; +char OffsetOverflowError::ID; +char DescriptorRangeOverflowError::ID; + template char RootSignatureValidationError::ID; static std::optional extractMdIntValue(MDNode *Node, @@ -547,21 +550,25 @@ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); - if (verifyOffsetOverflow(Range.OffsetInDescriptorsFromTableStart, - AppendingRegister)) - return make_error( + uint64_t StartSlot = AppendingRegister; + if (Range.OffsetInDescriptorsFromTableStart != ~0U) + StartSlot = Range.OffsetInDescriptorsFromTableStart; + + if (verifyOffsetOverflow(StartSlot)) + return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); if (verifyRegisterOverflow(Range.BaseShaderRegister, Range.NumDescriptors)) - return make_error( + return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); - if (verifyRegisterOverflow(AppendingRegister, Range.NumDescriptors)) - return make_error( + if (verifyRegisterOverflow(StartSlot, Range.NumDescriptors)) + return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); AppendingRegister = - updateAppendingRegister(AppendingRegister, Range.NumDescriptors); + updateAppendingRegister(StartSlot, Range.NumDescriptors, + Range.OffsetInDescriptorsFromTableStart); } return Error::success(); diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index ff420de392b94..36d6bd9d8b0a2 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -180,10 +180,7 @@ bool verifyBorderColor(uint32_t BorderColor) { bool verifyLOD(float LOD) { return !std::isnan(LOD); } -bool verifyOffsetOverflow(uint32_t Offset, uint64_t Register) { - if (Offset != ~0U) - Register = Offset; - +bool verifyOffsetOverflow(uint64_t Register) { if (Register > ~0U) return true; return false; @@ -201,11 +198,12 @@ bool verifyRegisterOverflow(uint64_t Register, uint32_t NumDescriptors) { return false; } -uint64_t updateAppendingRegister(uint64_t AppendingRegisterRegister, - uint32_t NumDescriptors) { +uint64_t updateAppendingRegister(uint64_t AppendingRegister, + uint64_t NumDescriptors, uint64_t Offset) { if (NumDescriptors == ~0U) return (uint64_t)~0U + (uint64_t)1ULL; - return AppendingRegisterRegister + NumDescriptors; + return Offset == ~0U ? AppendingRegister + NumDescriptors + : Offset + NumDescriptors; } } // namespace rootsig } // namespace hlsl diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll new file mode 100644 index 0000000000000..7517bc7c65121 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.ll @@ -0,0 +1,16 @@ +; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s +; CHECK: error: Overflow for descriptor range: UAV(register=0, space=0) +@TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 + +define void @CSMain() "hlsl.shader"="compute" { +entry: + ret void +} + +!dx.rootsignatures = !{!0} + +!0 = !{ptr @CSMain, !1, i32 2} +!1 = !{!3} +!3 = !{!"DescriptorTable", i32 0, !4, !5} +!4 = !{!"UAV", i32 100, i32 0, i32 0, i32 4294967294, i32 0} +!5 = !{!"UAV", i32 1, i32 101, i32 0, i32 10, i32 0} diff --git a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll index 65542a6ca98f2..935e0e146cecd 100644 --- a/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll +++ b/llvm/test/CodeGen/DirectX/rootsignature-validation-fail-register-overflow.ll @@ -1,5 +1,5 @@ ; RUN: not opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s -; CHECK: error: Cannot append range with implicit lower bound after an unbounded range UAV(register=4294967295, space=0). +; CHECK: error: Overflow for shader register range: UAV(register=4294967295, space=0) @TB.str = private unnamed_addr constant [3 x i8] c"TB\00", align 1 define void @CSMain() "hlsl.shader"="compute" { From e2ba167e14d1973ff81cbf454a042f13941b5ec0 Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Tue, 26 Aug 2025 11:10:38 -0700 Subject: [PATCH 17/18] removing StartSlot --- llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp index aa7a4d0e1e579..be79572ffed5d 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp @@ -550,11 +550,10 @@ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, dxbc::DescriptorRangeType RangeType = static_cast(Range.RangeType); - uint64_t StartSlot = AppendingRegister; if (Range.OffsetInDescriptorsFromTableStart != ~0U) - StartSlot = Range.OffsetInDescriptorsFromTableStart; + AppendingRegister = Range.OffsetInDescriptorsFromTableStart; - if (verifyOffsetOverflow(StartSlot)) + if (verifyOffsetOverflow(AppendingRegister)) return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); @@ -562,12 +561,12 @@ Error validateDescriptorTableRegisterOverflow(mcdxbc::DescriptorTable Table, return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); - if (verifyRegisterOverflow(StartSlot, Range.NumDescriptors)) + if (verifyRegisterOverflow(AppendingRegister, Range.NumDescriptors)) return make_error( RangeType, Range.BaseShaderRegister, Range.RegisterSpace); AppendingRegister = - updateAppendingRegister(StartSlot, Range.NumDescriptors, + updateAppendingRegister(AppendingRegister, Range.NumDescriptors, Range.OffsetInDescriptorsFromTableStart); } From b395a47ae90bdf300c15452293e5a1d3c059e9e1 Mon Sep 17 00:00:00 2001 From: Joao Saffran Date: Tue, 26 Aug 2025 11:29:16 -0700 Subject: [PATCH 18/18] removing unecessary code --- llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp index 36d6bd9d8b0a2..8d38166c8f021 100644 --- a/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp +++ b/llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp @@ -200,8 +200,7 @@ bool verifyRegisterOverflow(uint64_t Register, uint32_t NumDescriptors) { uint64_t updateAppendingRegister(uint64_t AppendingRegister, uint64_t NumDescriptors, uint64_t Offset) { - if (NumDescriptors == ~0U) - return (uint64_t)~0U + (uint64_t)1ULL; + return Offset == ~0U ? AppendingRegister + NumDescriptors : Offset + NumDescriptors; }