[DirectX] Adding missing descriptor table validations

llvm/include/llvm/Frontend/HLSL/RootSignatureMetadata.h

@@ -48,6 +48,91 @@ class RootSignatureValidationError
  }
 };
 
+class OffsetAppendAfterOverflow : public ErrorInfo<OffsetAppendAfterOverflow> {
+public:
+ static char ID;
+ dxil::ResourceClass Type;
+ uint32_t Register;
+ uint32_t Space;
+
+ OffsetAppendAfterOverflow(dxil::ResourceClass Type, uint32_t Register,
+ uint32_t Space)
+ : Type(Type), Register(Register), Space(Space) {}
+
+ void log(raw_ostream &OS) const override {
+ OS << "Range " << getResourceClassName(Type) << "(register=" << Register
+ << ", space=" << Space << ") "
+ << "cannot be appended after an unbounded range ";
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+class ShaderRegisterOverflowError
+ : public ErrorInfo<ShaderRegisterOverflowError> {
+public:
+ static char ID;
+ dxil::ResourceClass Type;
+ uint32_t Register;
+ uint32_t Space;
+
+ ShaderRegisterOverflowError(dxil::ResourceClass 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(Type)
+ << "(register=" << Register << ", space=" << Space << ").";
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+class OffsetOverflowError : public ErrorInfo<OffsetOverflowError> {
+public:
+ static char ID;
+ dxil::ResourceClass Type;
+ uint32_t Register;
+ uint32_t Space;
+
+ OffsetOverflowError(dxil::ResourceClass Type, uint32_t Register,
+ uint32_t Space)
+ : Type(Type), Register(Register), Space(Space) {}
+
+ void log(raw_ostream &OS) const override {
+ OS << "Offset overflow for descriptor range: " << getResourceClassName(Type)
+ << "(register=" << Register << ", space=" << Space << ").";
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
+class TableSamplerMixinError : public ErrorInfo<TableSamplerMixinError> {
+public:
+ static char ID;
+ dxil::ResourceClass Type;
+ uint32_t Location;
+
+ TableSamplerMixinError(dxil::ResourceClass 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(Type)
+ << "(location=" << Location << ")";
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+};
+
 class GenericRSMetadataError : public ErrorInfo<GenericRSMetadataError> {
 public:
  LLVM_ABI static char ID;

llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp

@@ -27,6 +27,11 @@ namespace rootsig {
 char GenericRSMetadataError::ID;
 char InvalidRSMetadataFormat::ID;
 char InvalidRSMetadataValue::ID;
+char TableSamplerMixinError::ID;
+char ShaderRegisterOverflowError::ID;
+char OffsetOverflowError::ID;
+char OffsetAppendAfterOverflow::ID;
+
 template <typename T> char RootSignatureValidationError<T>::ID;
 
 static std::optional<uint32_t> extractMdIntValue(MDNode *Node,
@@ -55,8 +60,9 @@ static std::optional<StringRef> extractMdStringValue(MDNode *Node,
 template <typename T, typename = std::enable_if_t<
  std::is_enum_v<T> &&
  std::is_same_v<std::underlying_type_t<T>, uint32_t>>>
-Expected<T> extractEnumValue(MDNode *Node, unsigned int OpId, StringRef ErrText,
- llvm::function_ref<bool(uint32_t)> VerifyFn) {
+static Expected<T>
+extractEnumValue(MDNode *Node, unsigned int OpId, StringRef ErrText,
+ llvm::function_ref<bool(uint32_t)> VerifyFn) {
  if (std::optional<uint32_t> Val = extractMdIntValue(Node, OpId)) {
  if (!VerifyFn(*Val))
  return make_error<RootSignatureValidationError<uint32_t>>(ErrText, *Val);
@@ -538,6 +544,60 @@ Error MetadataParser::parseRootSignatureElement(mcdxbc::RootSignatureDesc &RSD,
  llvm_unreachable("Unhandled RootSignatureElementKind enum.");
 }
 
+static Error
+validateDescriptorTableSamplerMixin(const mcdxbc::DescriptorTable &Table,
+ uint32_t Location) {
+ dxil::ResourceClass CurrRC = dxil::ResourceClass::Sampler;
+ for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
+ if (Range.RangeType == dxil::ResourceClass::Sampler &&
+ CurrRC != dxil::ResourceClass::Sampler)
+ return make_error<TableSamplerMixinError>(CurrRC, Location);
+ CurrRC = Range.RangeType;
+ }
+ return Error::success();
+}
+
+static Error
+validateDescriptorTableRegisterOverflow(const mcdxbc::DescriptorTable &Table,
+ uint32_t Location) {
+ uint64_t Offset = 0;
+ bool IsPrevUnbound = false;
+ for (const mcdxbc::DescriptorRange &Range : Table.Ranges) {
+ // Validation of NumDescriptors should have happened by this point.
+ if (Range.NumDescriptors == 0)
+ continue;
+
+ const uint64_t RangeBound = llvm::hlsl::rootsig::computeRangeBound(
+ Range.BaseShaderRegister, Range.NumDescriptors);
+
+ if (!verifyNoOverflowedOffset(RangeBound))
+ return make_error<ShaderRegisterOverflowError>(
+ Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+ bool IsAppending =
+ Range.OffsetInDescriptorsFromTableStart == DescriptorTableOffsetAppend;
+ if (!IsAppending)
+ Offset = Range.OffsetInDescriptorsFromTableStart;
+
+ if (IsPrevUnbound && IsAppending)
+ return make_error<OffsetAppendAfterOverflow>(
+ Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+ const uint64_t OffsetBound =
+ llvm::hlsl::rootsig::computeRangeBound(Offset, Range.NumDescriptors);
+
+ if (!verifyNoOverflowedOffset(OffsetBound))
+ return make_error<OffsetOverflowError>(
+ Range.RangeType, Range.BaseShaderRegister, Range.RegisterSpace);
+
+ Offset = OffsetBound + 1;
+ IsPrevUnbound =
+ Range.NumDescriptors == llvm::hlsl::rootsig::NumDescriptorsUnbounded;
+ }
+
+ return Error::success();
+}
+
 Error MetadataParser::validateRootSignature(
  const mcdxbc::RootSignatureDesc &RSD) {
  Error DeferredErrs = Error::success();
@@ -611,6 +671,14 @@ Error MetadataParser::validateRootSignature(
  joinErrors(std::move(DeferredErrs),
  make_error<RootSignatureValidationError<uint32_t>>(
  "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;
  }

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

@@ -136,7 +136,6 @@ uint64_t computeRangeBound(uint64_t Offset, uint32_t Size) {
 
  return Offset + uint64_t(Size) - 1;
 }
-
 } // namespace rootsig
 } // namespace hlsl
 } // namespace llvm

llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinations.ll

@@ -11,8 +11,9 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3, i32 2 } ; function, root signature
-!3 = !{ !5 } ; list of root signature elements
-!5 = !{ !"DescriptorTable", i32 0, !6, !8, !9, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
+!3 = !{ !5, !21 } ; list of root signature elements
+!5 = !{ !"DescriptorTable", i32 0, !10, !11, !12, !13, !14, !15, !16, !17, !18, !19, !20 }
+!21 = !{ !"DescriptorTable", i32 0, !6, !8, !9 }
 
 ; typedef enum D3D12_DESCRIPTOR_RANGE_FLAGS {
 ; NONE = 0,
@@ -53,37 +54,20 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !20 = !{ !"UAV", i32 5, i32 1, i32 15, i32 5, i32 65540 }
 
 ;DXC:- Name: RTS0
-;DXC-NEXT: Size: 380
+;DXC-NEXT: Size: 400
 ;DXC-NEXT: RootSignature:
 ;DXC-NEXT: Version: 2
-;DXC-NEXT: NumRootParameters: 1
+;DXC-NEXT: NumRootParameters: 2
 ;DXC-NEXT: RootParametersOffset: 24
 ;DXC-NEXT: NumStaticSamplers: 0
-;DXC-NEXT: StaticSamplersOffset: 380
+;DXC-NEXT: StaticSamplersOffset: 400
 ;DXC-NEXT: Parameters:
 ;DXC-NEXT: - ParameterType: DescriptorTable
 ;DXC-NEXT: ShaderVisibility: All
 ;DXC-NEXT: Table:
-;DXC-NEXT: NumRanges: 14
-;DXC-NEXT: RangesOffset: 44
+;DXC-NEXT: NumRanges: 11
+;DXC-NEXT: RangesOffset: 56
 ;DXC-NEXT: Ranges:
-;DXC-NEXT: - RangeType: Sampler
-;DXC-NEXT: NumDescriptors: 1
-;DXC-NEXT: BaseShaderRegister: 0
-;DXC-NEXT: RegisterSpace: 1
-;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT: - RangeType: Sampler
-;DXC-NEXT: NumDescriptors: 1
-;DXC-NEXT: BaseShaderRegister: 0
-;DXC-NEXT: RegisterSpace: 3
-;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT: DESCRIPTORS_VOLATILE: true
-;DXC-NEXT: - RangeType: Sampler
-;DXC-NEXT: NumDescriptors: 1
-;DXC-NEXT: BaseShaderRegister: 0
-;DXC-NEXT: RegisterSpace: 4
-;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
-;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
 ;DXC-NEXT: - RangeType: SRV
 ;DXC-NEXT: NumDescriptors: 1
 ;DXC-NEXT: BaseShaderRegister: 0
@@ -155,3 +139,26 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ;DXC-NEXT: OffsetInDescriptorsFromTableStart: 5
 ;DXC-NEXT: DATA_STATIC_WHILE_SET_AT_EXECUTE: true
 ;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true
+;DXC-NEXT: - ParameterType: DescriptorTable
+;DXC-NEXT: ShaderVisibility: All
+;DXC-NEXT: Table:
+;DXC-NEXT: NumRanges: 3
+;DXC-NEXT: RangesOffset: 328
+;DXC-NEXT: Ranges:
+;DXC-NEXT: - RangeType: Sampler
+;DXC-NEXT: NumDescriptors: 1
+;DXC-NEXT: BaseShaderRegister: 0
+;DXC-NEXT: RegisterSpace: 1
+;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT: - RangeType: Sampler
+;DXC-NEXT: NumDescriptors: 1
+;DXC-NEXT: BaseShaderRegister: 0
+;DXC-NEXT: RegisterSpace: 3
+;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT: DESCRIPTORS_VOLATILE: true
+;DXC-NEXT: - RangeType: Sampler
+;DXC-NEXT: NumDescriptors: 1
+;DXC-NEXT: BaseShaderRegister: 0
+;DXC-NEXT: RegisterSpace: 4
+;DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
+;DXC-NEXT: DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: true

llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll

@@ -11,33 +11,40 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
-!3 = !{ !5 } ; list of root signature elements
-!5 = !{ !"DescriptorTable", i32 0, !6, !7 }
+!3 = !{ !5, !8 } ; list of root signature elements
+!5 = !{ !"DescriptorTable", i32 0, !6 }
 !6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
+!8 = !{ !"DescriptorTable", i32 0, !7 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
 
 
 ; DXC: - Name: RTS0
-; DXC-NEXT: Size: 84
+; DXC-NEXT: Size: 104
 ; DXC-NEXT: RootSignature:
 ; DXC-NEXT: Version: 1
-; DXC-NEXT: NumRootParameters: 1
+; DXC-NEXT: NumRootParameters: 2
 ; DXC-NEXT: RootParametersOffset: 24
 ; DXC-NEXT: NumStaticSamplers: 0
-; DXC-NEXT: StaticSamplersOffset: 84
+; DXC-NEXT: StaticSamplersOffset: 104
 ; DXC-NEXT: Parameters:
 ; DXC-NEXT: - ParameterType: DescriptorTable
 ; DXC-NEXT: ShaderVisibility: All
 ; DXC-NEXT: Table:
-; DXC-NEXT: NumRanges: 2
-; DXC-NEXT: RangesOffset: 44
+; DXC-NEXT: NumRanges: 1
+; DXC-NEXT: RangesOffset: 56
 ; DXC-NEXT: Ranges:
 ; DXC-NEXT: - RangeType: Sampler
 ; DXC-NEXT: NumDescriptors: 1
 ; DXC-NEXT: BaseShaderRegister: 1
 ; DXC-NEXT: RegisterSpace: 0
 ; DXC-NEXT: OffsetInDescriptorsFromTableStart: 4294967295
-; DXC-NEXT: - RangeType: UAV
+; DXC-NEXT: - ParameterType: DescriptorTable
+; DXC-NEXT: ShaderVisibility: All
+; DXC-NEXT: Table:
+; DXC-NEXT: NumRanges: 1
+; DXC-NEXT: RangesOffset: 84
+; DXC-NEXT: Ranges:
+; DXC-NEXT: - RangeType: UAV
 ; DXC-NEXT: NumDescriptors: 5
 ; DXC-NEXT: BaseShaderRegister: 1
 ; DXC-NEXT: RegisterSpace: 10

llvm/test/CodeGen/DirectX/rootsignature-validation-appending-limits.ll

@@ -0,0 +1,16 @@
+; RUN: opt -S -passes='dxil-post-optimization-validation' -mtriple=dxil-pc-shadermodel6.6-compute %s 2>&1 | FileCheck %s
+; A descriptor range can be placed at UINT_MAX, matching DXC's behaviour 
+; CHECK-NOT: error:
+
+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 0, i32 4294967294, i32 0}
+!5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0}

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-limits-multiples.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: Offset overflow for descriptor range: CBV(register=2, space=0).
+
+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, !6}
+!4 = !{!"CBV", i32 1, i32 0, i32 0, i32 4294967294, i32 0}
+!5 = !{!"CBV", i32 1, i32 1, i32 0, i32 -1, i32 0}
+!6 = !{!"CBV", i32 1, i32 2, i32 0, i32 -1, i32 0}

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-appending-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
+; This test checks if a resource is implicitly overflowing. That means, it is appending a resource after an unbounded range. 
+
+; CHECK: error: Range UAV(register=0, space=0) cannot be appended after an unbounded range
+
+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 0, i32 2, i32 0}
+!5 = !{!"UAV", i32 1, i32 0, i32 0, i32 -1, i32 0}

llvm/test/CodeGen/DirectX/rootsignature-validation-fail-offset-overflow.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: Offset overflow for descriptor range: UAV(register=0, space=0).
+
+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}